[PATCH] Make x265 build on ppc64le

  • Done
  • quality assurance status badge
Details
3 participants
  • Thiago Jung Bauermann
  • Marcel van der Boom
  • Mathieu Othacehe
Owner
unassigned
Submitted by
Marcel van der Boom
Severity
normal

Debbugs page

Marcel van der Boom wrote 3 years ago
(address . guix-patches@gnu.org)(name . Marcel van der Boom)(address . marcel@van-der-boom.nl)
f7be730ee74999da0f20b44c94c805681a194730.1659442047.git.marcel@van-der-boom.nl
* gnu/packages/video.scm: disable ALTIVEC for target-ppc64le
---
gnu/packages/video.scm | 6 ++++++
1 file changed, 6 insertions(+)

Toggle diff (28 lines)
diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 04049fd9c8..09f8b7fd23 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -1252,6 +1252,9 @@ (define-public x265
,@(if (target-aarch64?)
'("-DENABLE_ASSEMBLY=OFF")
'())
+ ,@(if (target-ppc64le?)
+ '("-DENABLE_ALTIVEC=OFF")
+ '())
"-DHIGH_BIT_DEPTH=ON"
"-DEXPORT_C_API=OFF"
"-DENABLE_CLI=OFF"
@@ -1272,6 +1275,9 @@ (define-public x265
,@(if (target-aarch64?)
'("-DENABLE_ASSEMBLY=OFF")
'())
+ ,@(if (target-ppc64le?)
+ '("-DENABLE_ALTIVEC=OFF")
+ '())
"-DHIGH_BIT_DEPTH=ON"
"-DEXPORT_C_API=OFF"
"-DENABLE_CLI=OFF"

base-commit: a1f7d98a9c7380be5815fd13b519bbab145757c2
--
2.37.1
Marcel van der Boom wrote 3 years ago
(address . 56884@debbugs.gnu.org)
87wnbhder7.fsf@van-der-boom.nl
Anything else needed here?

The patch seems consistent with other packagers I think. Others
are also confused on what is needed for the altivec code between
different POWER processors.
Thiago Jung Bauermann wrote 3 years ago
(name . Marcel van der Boom)(address . marcel@van-der-boom.nl)(address . 56884@debbugs.gnu.org)
87r11oddt9.fsf@kolabnow.com
Hello Marcel,

Marcel van der Boom <marcel@van-der-boom.nl> writes:

Toggle quote (4 lines)
> Anything else needed here?
>
> The patch seems consistent with other packagers I think.

Thank you for the patch! It looks good to me. I only have one
suggestion: IMHO it would be a good idea to have a short comment above
the “(if (target-ppc64le?) …)” line explaining why it's needed, such as
“Enabling AltiVec support causes compilation errors.” or something
similar.

Ideally upstream should be notified about this problem so that they are
aware of it, but looking at the x265 website I can't find any bug
tracker.

Toggle quote (3 lines)
> Others are also confused on what is needed for the altivec code
> between different POWER processors.

Sorry, I'm not sure what you mean.

PS: Just for the sake of documenting in the issue tracker why x265 isn't
building on ppc64le — the build fails because of many errors such as:

/tmp/guix-build-x265-3.5.drv-0/x265_3.5/source/common/ppc/pixel_altivec.cpp:4199:42: error: no matches converting function ‘sad16_altivec’ to type ‘x265_12bit::pixelcmp_t’ {aka ‘int (*)(const short unsigned int*, long int, const short unsigned int*, long int)’}
4199 | p.pu[LUMA_ ## W ## x ## H].sad = sad16_altivec<W, H>; \
| ^~~~~~~~~~~~~~~~~~~

--
Thanks
Thiago
Marcel van der Boom wrote 3 years ago
[PATCH] Make x265 build on ppc64le
(address . 56884@debbugs.gnu.org)(name . Marcel van der Boom)(address . marcel@van-der-boom.nl)(name . Thiago Jung Bauermann)(address . bauermann@kolabnow.com)
097e5d484b14e0777f26bfd5c34b76189b828b8c.1660115870.git.marcel@van-der-boom.nl
* gnu/packages/video.scm: disable ALTIVEC for target-ppc64le

Build produces many errors. The altivec code needs adjusting each type of
POWER processor specifically.
---
gnu/packages/video.scm | 8 ++++++++
1 file changed, 8 insertions(+)

Toggle diff (28 lines)
diff --git a/gnu/packages/video.scm b/gnu/packages/video.scm
index 04049fd9c8..bf033c7d84 100644
--- a/gnu/packages/video.scm
+++ b/gnu/packages/video.scm
@@ -1252,6 +1252,10 @@ (define-public x265
,@(if (target-aarch64?)
'("-DENABLE_ASSEMBLY=OFF")
'())
+ ;; Altivec code produces many build errors
+ ,@(if (target-ppc64le?)
+ '("-DENABLE_ALTIVEC=OFF")
+ '())
"-DHIGH_BIT_DEPTH=ON"
"-DEXPORT_C_API=OFF"
"-DENABLE_CLI=OFF"
@@ -1272,6 +1276,10 @@ (define-public x265
,@(if (target-aarch64?)
'("-DENABLE_ASSEMBLY=OFF")
'())
+ ;; Altivec code produces many build errors
+ ,@(if (target-ppc64le?)
+ '("-DENABLE_ALTIVEC=OFF")
+ '())
"-DHIGH_BIT_DEPTH=ON"
"-DEXPORT_C_API=OFF"
"-DENABLE_CLI=OFF"
--
2.37.1
Thiago Jung Bauermann wrote 3 years ago
(name . Marcel van der Boom)(address . marcel@van-der-boom.nl)(address . 56884@debbugs.gnu.org)
87edxnjzdn.fsf@kolabnow.com
user guix
usertag 56884 reviewed looks-good
quit

Hello Marcel,

Marcel van der Boom <marcel@van-der-boom.nl> writes:

Toggle quote (5 lines)
> * gnu/packages/video.scm: disable ALTIVEC for target-ppc64le
>
> Build produces many errors. The altivec code needs adjusting each type of
> POWER processor specifically.

Thank you for the new version. This one looks good to me.

I'm not a maintainer though, so we need to wait for one to chime in.

--
Thanks
Thiago
Mathieu Othacehe wrote 3 years ago
Re: bug#56884: [PATCH] Make x265 build on ppc64le
(name . Thiago Jung Bauermann)(address . bauermann@kolabnow.com)(address . 56884-done@debbugs.gnu.org)(name . Marcel van der Boom)(address . marcel@van-der-boom.nl)
87h72j440m.fsf_-_@gnu.org
Hello,

Thanks for reviewing Thiago. I fixed the indentation and edited the
commit message before pushing.

Mathieu
Closed
Thiago Jung Bauermann wrote 3 years ago
(name . Mathieu Othacehe)(address . othacehe@gnu.org)(address . 56884-done@debbugs.gnu.org)(name . Marcel van der Boom)(address . marcel@van-der-boom.nl)
87pmh6g7fu.fsf@kolabnow.com
Hello Mathieu,

Mathieu Othacehe <othacehe@gnu.org> writes:

Toggle quote (3 lines)
> Thanks for reviewing Thiago. I fixed the indentation and edited the
> commit message before pushing.

Thank you!

--
Thanks
Thiago
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 56884@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 56884
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help