Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[Fernando Seiti Furusato]

J Price <j.pr...@bristol.ac.uk> wrote on 08/12/2015 19:14:00:

> From: J Price <j.pr...@bristol.ac.uk>
> To: Fernando Seiti Furusato/Brazil/IBM@IBMBR
> Cc: Andreas Beckmann <a...@debian.org>, 806...@bugs.debian.org, Breno 
Henrique
> Leitao/Brazil/IBM@IBMBR
> Date: 08/12/2015 19:14
> Subject: Re: Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with 
altivec 
> keyword bool
> 
> You're right - this needs to be done in common.h for this to work
> (since common.h uses the bool keyword), but common.h is installed by
> the -dev package, so this goes against the idea of not messing with
> macros in public headers.

Oh, I should have noticed that.

> 
> Reading some other bug reports (e.g. [1]) suggests that compiling with
> `-std=gnu++11` instead of `-std=c++11` also fixes the problem. Is this
> a viable workaround? This would be a simple patch to CMakeLists.txt.

In case this question was directed to me -- it is viable in the sense that 
it
works. Just tested it here and it built.

Thanks and regards.

Fernando

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[J Price]

On 9 December 2015 at 13:17, Fernando Seiti Furusato
 wrote:
>
>> Reading some other bug reports (e.g. [1]) suggests that compiling with
>> `-std=gnu++11` instead of `-std=c++11` also fixes the problem. Is this
>> a viable workaround? This would be a simple patch to CMakeLists.txt.
>
> In case this question was directed to me -- it is viable in the sense that
> it
> works. Just tested it here and it built.

OK, if Andreas is happy with this fix I'll commit this instead.

James

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[Andreas Beckmann]

On 2015-12-09 17:18, J Price wrote:
> On 9 December 2015 at 13:17, Fernando Seiti Furusato
>  wrote:
>>
>>> Reading some other bug reports (e.g. [1]) suggests that compiling with
>>> `-std=gnu++11` instead of `-std=c++11` also fixes the problem. Is this
>>> a viable workaround? This would be a simple patch to CMakeLists.txt.
>>
>> In case this question was directed to me -- it is viable in the sense that
>> it
>> works. Just tested it here and it built.
> 
> OK, if Andreas is happy with this fix I'll commit this instead.

Sounds good!

Andreas

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[Fernando Seiti Furusato]

Hello, James.

J Price <j.pr...@bristol.ac.uk> wrote on 07/12/2015 22:32:48:

> From: J Price <j.pr...@bristol.ac.uk>
> To: Fernando Seiti Furusato/Brazil/IBM@IBMBR
> Cc: Andreas Beckmann <a...@debian.org>, 806...@bugs.debian.org, 
cont...@bugs.debian.org
> Date: 07/12/2015 22:32
> Subject: Re: Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with 
altivec 
> keyword bool
> 
> OK, the ppc64el build is still failing with the __vector patch. It's
> not clear to me where the errors are originating.
> 
The errors are pretty much the same.
But the problem is with keyword bool, not vector.

> Perhaps we need to #undef these macros in cl.h after  is
> included, to prevent them from causing problems in the source files
> that include cl.h?
Yes, the solution would be the same used with __vector, but for bool as 
well.
I can test it here. I will let you know.

Thanks.

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[Fernando Seiti Furusato]

Fernando Seiti Furusato/Brazil/IBM wrote on 08/12/2015 09:13:26:

> From: Fernando Seiti Furusato/Brazil/IBM
> To: J Price <j.pr...@bristol.ac.uk>
> Cc: 806...@bugs.debian.org, Andreas Beckmann <a...@debian.org>, 
> cont...@bugs.debian.org, Breno Henrique Leitao/Brazil/IBM@IBMBR
> Date: 08/12/2015 09:13
> Subject: Re: Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with 
altivec 
> keyword bool
> 
> The errors are pretty much the same.
> But the problem is with keyword bool, not vector.

Turns out it is not as simple.
What helped a lot with vector is that it is being used to define a type
in a header, unlike bool which is used everywhere.
Also the first error is because it is used in an enum in a gcc header.
(/usr/include/c++/5/bits/cpp_type_traits.h)

Maybe it is worth (or possible?) trying to compile the parts that result
in error with the -mno-altivec part.
I am not sure how it all works, let me investigate further.

Regards

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[Fernando Seiti Furusato]

By the way, do you want me to create a patch, or you will do that directly
upstream?

Cheers.

Fernando

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[J Price]

On 8 December 2015 at 17:40, Fernando Seiti Furusato
 wrote:
> By the way, do you want me to create a patch, or you will do that directly
> upstream?

I'll do this in the package SCM now and request another upload.

James

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[J Price]

On 8 December 2015 at 13:43, Fernando Seiti Furusato
 wrote:
> What helped a lot with vector is that it is being used to define a type
> in a header, unlike bool which is used everywhere.
> Also the first error is because it is used in an enum in a gcc header.
> (/usr/include/c++/5/bits/cpp_type_traits.h)

Since bool is just a macro, can't we just #undef it after including
altivec.h? I was envisioning something like this inside cl_platform.h
(around line 334):

#include 
#undef vector
#undef bool
#undef pixel
typedef __vector unsigned char  __cl_uchar16;
// etc

James

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[Andreas Beckmann]

On 2015-12-08 18:42, J Price wrote:
> On 8 December 2015 at 17:40, Fernando Seiti Furusato
>  wrote:
>> By the way, do you want me to create a patch, or you will do that directly
>> upstream?
> 
> I'll do this in the package SCM now and request another upload.

BTW, why do you need your own copy of the opencl headers at all? Can't
you just use the headers from opencl-headers? (I probably asked this
before ...)
I don't really like the idea of manipulating unrelated defines in
headers ...

Isn't that a problem for many projects?

#include   // #define bool ...
#include   // explode on bool macro


Andreas

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[Fernando Seiti Furusato]

J Price <j.pr...@bristol.ac.uk> wrote on 08/12/2015 15:30:55:

> From: J Price <j.pr...@bristol.ac.uk>
> To: Fernando Seiti Furusato/Brazil/IBM@IBMBR
> Cc: 806...@bugs.debian.org, Andreas Beckmann <a...@debian.org>, Breno 
Henrique
> Leitao/Brazil/IBM@IBMBR
> Date: 08/12/2015 15:31
> Subject: Re: Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with 
altivec 
> keyword bool

> Since bool is just a macro, can't we just #undef it after including
> altivec.h? I was envisioning something like this inside cl_platform.h
> (around line 334):
> 
> #include 
> #undef vector
> #undef bool
> #undef pixel
> typedef __vector unsigned char  __cl_uchar16;
> // etc
> 

You are right, James.
That worked like a charm.
I did that and it completed with no errors :)

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[Andreas Beckmann]

On 2015-12-08 20:00, J Price wrote:
>> I don't really like the idea of manipulating unrelated defines in
>> headers ...
> 
> OK, so how about if I do this from the .cpp files instead? e.g.

That would be fine, since it's limited to oclgrind. Doing it in a
(public) header could "contaminate" other projects that include the
header, also it may depend on the order of the includes whether
compilation fails or succeeds.

> 
> (in foo.cpp)
> #include 
#ifdef FOO
> #undef vector
> #undef bool
> #undef pixel
#endif

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[J Price]

On 8 December 2015 at 18:28, Andreas Beckmann  wrote:
> BTW, why do you need your own copy of the opencl headers at all? Can't
> you just use the headers from opencl-headers? (I probably asked this
> before ...)

You're right. Upstream needs the local copy since many systems don't
have recent enough OpenCL headers, but for Debian I could add a patch
to remove the local copy and have a build-dependency on opencl-headers
instead.


> I don't really like the idea of manipulating unrelated defines in
> headers ...

OK, so how about if I do this from the .cpp files instead? e.g.

(in foo.cpp)
#include 
#undef vector
#undef bool
#undef pixel


James

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[Fernando Seiti Furusato]

Andreas Beckmann <a...@debian.org> wrote on 08/12/2015 17:09:58:

> From: Andreas Beckmann <a...@debian.org>
> To: J Price <j.pr...@bristol.ac.uk>
> Cc: Fernando Seiti Furusato/Brazil/IBM@IBMBR, 806...@bugs.debian.org, 
Breno 
> Henrique Leitao/Brazil/IBM@IBMBR
> Date: 08/12/2015 17:10
> Subject: Re: Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with 
altivec 
> keyword bool
> 
> On 2015-12-08 20:00, J Price wrote:
> >> I don't really like the idea of manipulating unrelated defines in
> >> headers ...
> > 
> > OK, so how about if I do this from the .cpp files instead? e.g.
> 
> That would be fine, since it's limited to oclgrind. Doing it in a
> (public) header could "contaminate" other projects that include the
> header, also it may depend on the order of the includes whether
> compilation fails or succeeds.
> 

Just FYI.
Not sure if they are public headers or not, but doing so on headers:
src/runtime/icd.h
src/core/common.h

Made it build for me.

Right below #include "CL/cl.h" I included this:

#ifdef __ALTIVEC__
# undef bool
# undef vector
# undef pixel
#endif

I tried adding to the .cpp files (not all of them) but it didn't work.
The headers above are the ones that actually use cl.h.

Regards.

Fernando

Bug#806965: [Pkg-opencl-devel] Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[Rebecca N. Palmer]

You're right. Upstream needs the local copy since many systems don't
have recent enough OpenCL headers, but for Debian I could add a patch
to remove the local copy and have a build-dependency on opencl-headers
instead.


opencl-headers already has the __vector fix, but as already noted, the 
bool problem is a deeper "this is a keyword in standard C++" issue:


http://sources.debian.net/src/khronos-opencl-headers/2.0~svn32091-1/debian/patches/use__vector.patch

On 08/12/15 21:14, J Price wrote:

Reading some other bug reports (e.g. [1]) suggests that compiling with
`-std=gnu++11` instead of `-std=c++11` also fixes the problem. Is this
a viable workaround? This would be a simple patch to CMakeLists.txt.


Probably; if you want to test on ppc64el there's qemu-debootstrap (~10x 
slower, but shouldn't be a problem for a package this small), 
https://wiki.debian.org/CrossCompiling or the porterbox, but several of 
my FTBFS-on-$arch fixes were uploaded without testing on $arch.

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[J Price]

On 8 December 2015 at 19:52, Fernando Seiti Furusato
 wrote:
> Just FYI.
> Not sure if they are public headers or not, but doing so on headers:
> src/runtime/icd.h
> src/core/common.h
>
> Made it build for me.
>
> Right below #include "CL/cl.h" I included this:
>
> #ifdef __ALTIVEC__
> # undef bool
> # undef vector
> # undef pixel
> #endif
>
> I tried adding to the .cpp files (not all of them) but it didn't work.
> The headers above are the ones that actually use cl.h.

You're right - this needs to be done in common.h for this to work
(since common.h uses the bool keyword), but common.h is installed by
the -dev package, so this goes against the idea of not messing with
macros in public headers.

Reading some other bug reports (e.g. [1]) suggests that compiling with
`-std=gnu++11` instead of `-std=c++11` also fixes the problem. Is this
a viable workaround? This would be a simple patch to CMakeLists.txt.

[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=58241

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[Fernando Seiti Furusato]

Hello, gentlemen.

Andreas Beckmann  wrote on 04/12/2015 22:04:38:

> James,
> 
> you need to keep the original bug submitter Cc:ed  if you want them to
> see your reply.

Not to worry. I should have kept track of the bug report as well.

> On 2015-12-04 15:13, James Price wrote:
> > Thanks for the report. I’ve actually applied a slightly different fix 
for 
> this a little while back (see [1]), but that revision has yet to be 
uploaded.
> > 
> > The fix I applied was essentially the same as has been done for the 
opencl-
> headers package, described in [2]. This just uses __vector instead of 
vector 
> to avoid conflicts with similarly named types.
> > 
> > If you think the '-mno-altivec’ fix is preferable then I can revert my 
patch
> and apply yours instead. 
> 
> I think -mno-altivec is rather a quick workaround and __vector the
> proper fix.

Yes, I agree with Andreas. Mine was just a less invasive workaround, 
because
I preferred not to mess with the code.

> 
> > Otherwise I’ll just request that the latest revision be uploaded.
> 
> I'll take care of that next week.

Great, thanks!

--
Fernando Seiti Furusato
Software Engineer
IBM Brazil - Linux Technology Center

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[J Price]

reopen 806965
thanks

On 7 December 2015 at 11:08, Fernando Seiti Furusato
 wrote:
>
> > I think -mno-altivec is rather a quick workaround and __vector the
> > proper fix.
>
> Yes, I agree with Andreas. Mine was just a less invasive workaround,
> because I preferred not to mess with the code.

OK, the ppc64el build is still failing with the __vector patch. It's
not clear to me where the errors are originating.

>From the GCC page on AltiVec [1]:
> Macros vector, pixel, and bool are defined in  and can be 
> undefined.

Perhaps we need to #undef these macros in cl.h after  is
included, to prevent them from causing problems in the source files
that include cl.h?

[1] 
https://gcc.gnu.org/onlinedocs/gcc-4.2.4/gcc/PowerPC-AltiVec-Built_002din-Functions.html

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[James Price]

Thanks for the report. I’ve actually applied a slightly different fix for this 
a little while back (see [1]), but that revision has yet to be uploaded.

The fix I applied was essentially the same as has been done for the 
opencl-headers package, described in [2]. This just uses __vector instead of 
vector to avoid conflicts with similarly named types.

If you think the '-mno-altivec’ fix is preferable then I can revert my patch 
and apply yours instead. Otherwise I’ll just request that the latest revision 
be uploaded.

Many thanks,

James

[1] 
https://anonscm.debian.org/cgit/pkg-opencl/oclgrind.git/commit/?id=cd9925f14b76d3867166b524b3305ae156f5ef6b
 

[2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=760310 

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[Andreas Beckmann]

James,

you need to keep the original bug submitter Cc:ed  if you want them to
see your reply.

On 2015-12-04 15:13, James Price wrote:
> Thanks for the report. I’ve actually applied a slightly different fix for 
> this a little while back (see [1]), but that revision has yet to be uploaded.
> 
> The fix I applied was essentially the same as has been done for the 
> opencl-headers package, described in [2]. This just uses __vector instead of 
> vector to avoid conflicts with similarly named types.
> 
> If you think the '-mno-altivec’ fix is preferable then I can revert my patch 
> and apply yours instead. 

I think -mno-altivec is rather a quick workaround and __vector the
proper fix.

> Otherwise I’ll just request that the latest revision be uploaded.

I'll take care of that next week.

> 
> Many thanks,
> 
> James
> 
> [1] 
> https://anonscm.debian.org/cgit/pkg-opencl/oclgrind.git/commit/?id=cd9925f14b76d3867166b524b3305ae156f5ef6b
>  
> 
> [2] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=760310 
> 

Andreas

Bug#806965: oclgrind: FTBFS on ppc64el -- conflict with altivec keyword bool

[Fernando Seiti Furusato]

Source: oclgrind
Version: 15.5-1
Severity: normal
Tags: patch

Dear Maintainer,

The package oclgrind fails to build from source on ppc64el with,
initially the error:

  /usr/include/c++/5/bits/cpp_type_traits.h:98:24: error: expected 
primary-expression before '__attribute__'
 enum { __value = bool(_Sp::__value) || bool(_Tp::__value) };

Full log can be found at:
https://buildd.debian.org/status/fetch.php?pkg=oclgrind=ppc64el=15.5-1=1445511925

I created a patch to bypass usage of altivec with -mno-altivec flag.
The concerns on using the method of the patch I created is that it will
not apply on cross-compiling and it will apply for the whole code, not just
for the parts where it fails.

In any case, the package builds successfully with the patch.
If you have any concerns, please let me know.

Thanks and regards.

Fernando
diff -Nru oclgrind-15.5/debian/changelog oclgrind-15.5/debian/changelog
--- oclgrind-15.5/debian/changelog	2015-09-12 13:22:48.0 -0400
+++ oclgrind-15.5/debian/changelog	2015-12-03 10:20:43.0 -0500
@@ -1,3 +1,10 @@
+oclgrind (15.5-1.1) UNRELEASED; urgency=medium
+
+  * Non-maintainer upload.
+  * Added condition to not use altivec on ppc64el to fix FTBFS 
+
+ -- Fernando Seiti Furusato   Thu, 03 Dec 2015 10:20:26 -0500
+
 oclgrind (15.5-1) unstable; urgency=low
 
   * Initial release (Closes: #794787)
diff -Nru oclgrind-15.5/debian/patches/ppc64el-no-altivec.patch oclgrind-15.5/debian/patches/ppc64el-no-altivec.patch
--- oclgrind-15.5/debian/patches/ppc64el-no-altivec.patch	1969-12-31 19:00:00.0 -0500
+++ oclgrind-15.5/debian/patches/ppc64el-no-altivec.patch	2015-12-03 10:20:24.0 -0500
@@ -0,0 +1,11 @@
+--- a/CMakeLists.txt
 b/CMakeLists.txt
+@@ -299,3 +299,8 @@
+ 
+ # Add app tests
+ add_subdirectory(tests/apps)
++
++# Condition not to use altivec on ppc64el
++if (CMAKE_SYSTEM_PROCESSOR MATCHES "ppc64le")
++ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -mno-altivec")
++endif()
diff -Nru oclgrind-15.5/debian/patches/series oclgrind-15.5/debian/patches/series
--- oclgrind-15.5/debian/patches/series	2015-09-12 13:22:48.0 -0400
+++ oclgrind-15.5/debian/patches/series	2015-12-03 10:20:10.0 -0500
@@ -6,3 +6,4 @@
 pch-location.patch
 clang-library-order.patch
 cmake-preserve-flags.patch
+ppc64el-no-altivec.patch