Bug 91826

Summary: Mesa 11.0.0-rc2 - state_trackers/clover does not build on FreeBSD
Product: Mesa Reporter: Koop Mast <kwm>
Component: OtherAssignee: mesa-dev
Status: RESOLVED FIXED QA Contact: mesa-dev
Severity: normal    
Priority: medium    
Version: 11.0   
Hardware: x86-64 (AMD64)   
OS: FreeBSD   
Whiteboard:
i915 platform: i915 features:

Description Koop Mast 2015-08-31 18:51:48 UTC
Mesa 11.0.0 rc2 doesn't build on FreeBSD with the build failure below. Used compiler is Clang 3.6.1 and we use llvm's libc++ as c++ standard library.

This seems to be that clang is picky about this piece of code. A quick test shows that GCC 5.2.1 build the code just fine, even if it issues a warning about the issue clang errors on. 

Sorry my c++ know how is almost non existing, so can't help with making a patch for this issue.

Clang:
src/gallium/state_trackers/clover % gmake
  CXX      llvm/libclllvm_la-invocation.lo
llvm/invocation.cpp:388:12: warning: unused variable 'str_node'
      [-Wunused-variable]
      auto str_node = llvm::cast<llvm::MDString>(node->getOperand(0));
           ^
llvm/invocation.cpp:468:40: error: typename specifier refers to non-type member
      'type' in 'clover::module::argument'
            typename module::argument::type marg_type;
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
./core/module.hpp:97:15: note: referenced member 'type' is declared here
         type type;
              ^
1 warning and 1 error generated.
Makefile:859: recipe for target 'llvm/libclllvm_la-invocation.lo' failed
gmake: *** [llvm/libclllvm_la-invocation.lo] Error 1


GCC:
  CXXLD    libcltgsi.la
llvm/invocation.cpp: In function 'llvm::MDNode* {anonymous}::node_from_op_checked(const llvm::MDOperand&, llvm::StringRef, unsigned int)':
llvm/invocation.cpp:388:12: warning: unused variable 'str_node' [-Wunused-variable]
       auto str_node = llvm::cast<llvm::MDString>(node->getOperand(0));
            ^
In file included from ./core/compiler.hpp:27:0,
                 from llvm/invocation.cpp:23:
./core/module.hpp: In function 'std::vector<clover::module::argument> {anonymous}::get_kernel_args(const llvm::Module*, const string&, const unsigned int (&)[7])':
./core/module.hpp:86:50: warning: 'marg_type' may be used uninitialized in this function [-Wmaybe-uninitialized]
             ext_type(ext_type), semantic(semantic) { }
                                                  ^
llvm/invocation.cpp:468:45: note: 'marg_type' was declared here
             typename module::argument::type marg_type;
                                             ^
  CXXLD    libclllvm.la
Comment 1 Albert Freeman 2015-09-01 05:28:20 UTC
I believe the fix for this is in mesa master. Here is the patch: https://patchwork.freedesktop.org/patch/56716/
Comment 2 Albert Freeman 2015-09-01 05:41:48 UTC
Ignore my last comment, it is wrong, I got very confused.
Comment 3 Albert Freeman 2015-09-01 06:49:03 UTC
Well changing typename to enum seems to fix the problem:

diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
index 7c23a27..d74b50d 100644
--- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
+++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
@@ -465,7 +465,7 @@ namespace {
             const bool is_write_only = access_qual == "write_only";
             const bool is_read_only = access_qual == "read_only";
 
-            typename module::argument::type marg_type;
+            enum module::argument::type marg_type;
             if (is_image2d && is_read_only) {
                marg_type = module::argument::image2d_rd;
             } else if (is_image2d && is_write_only) {

Results in clang compilation warnings that are the equivalent of the gcc ones.

Of course someone familiar with the code should review this (especially since I only learned what typename meant within the last few hours (correction: I still don't really know)).
Comment 4 Albert Freeman 2015-09-01 06:50:26 UTC
You should probably report this to the clang developers too.
Comment 5 Koop Mast 2015-09-01 09:30:58 UTC
The patch from #3 fixes the build with clang, thanks.

Report what to clang upstream, that they error on this issue versus gcc which only gives a warning?
Comment 6 Albert Freeman 2015-09-01 09:33:46 UTC
Yeah, I don't know too much about C++11 but it MIGHT be a bug in clang.

On 1 September 2015 at 09:30,  <bugzilla-daemon@freedesktop.org> wrote:
> Comment # 5 on bug 91826 from Koop Mast
>
> The patch from #3 fixes the build with clang, thanks.
>
> Report what to clang upstream, that they error on this issue versus gcc
> which
> only gives a warning?
>
> ________________________________
> You are receiving this mail because:
>
> You are the QA Contact for the bug.
> You are the assignee for the bug.
>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
Comment 7 Koop Mast 2015-09-06 21:09:29 UTC
hi, this bug has been marked as fixed but the fix hasn't been committed yet, is there some hold up? Or should the bug be reopened until it ready to be committed?
Comment 8 Albert Freeman 2015-09-07 04:36:10 UTC
Sorry, I just realised this is my fault. The patch has been reviewed, but due to bad formatting of the subject line/commit message it has not been committed. The fix should be in by next release.
Comment 9 Emil Velikov 2015-09-10 14:01:39 UTC
Hi guys,

Just a friendly suggestion - please keep the bug open until the patch lands. This way we have a nice reminder if we forget/miss them :-)
To provide the link back to the bugreport -> add a tag Bugzilla: http.... in the commit message.

Thank you.
Comment 10 Albert Freeman 2015-09-10 16:00:53 UTC
Okay,

I read the mesa http://www.mesa3d.org/devinfo.html out of context, at that time I thought comment meant PATCH comment (below the dashes).

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.