build error: zero-size array ‘mattr’ with LLVM 3.5

[rekado]

I'm trying to upgrade from 0.3.6 to 0.3.9, but I get the following build error:

codegen.cpp: In function ‘void jl_init_codegen()’:
codegen.cpp:4554:5: error: zero-size array ‘mattr’
     };
     ^
    LINK src/flisp/libflisp.a
    LINK src/flisp/flisp
    FLISP src/julia_flisp.boot
    FLISP src/julia_flisp.boot.inc
Makefile:56: recipe for target 'codegen.o' failed
make[2]: *** [codegen.o] Error 1
make[2]: *** Waiting for unfinished jobs....
Makefile:90: recipe for target 'julia-release' failed
make[1]: *** [julia-release] Error 2
Makefile:37: recipe for target 'release' failed
make: *** [release] Error 2

I'm compiling with GCC4.9 using a minimally modified version of the Guix recipe here.

[tkelman]

b940fdf is at fault, not sure what the best patch is. Looks like you're building with LLVM 3.5?

[rekado]

Yes, I'm building with LLVM 3.5. I cannot use LLVM 3.6 because "llvm/ExecutionEngine/JITMemoryManager.h" is not provided by the 3.6 package.

[tkelman]

We haven't backported all fixes for building against LLVM 3.6 or svn; 3.3 (plus patches where necessary) is really the primary supported version for release-0.3.

Not sure why you're the first one to report a problem though. What's the easiest way to reproduce the dependency environment and build flags here? Could you write a dockerfile that quickly reproduces this inside guix?

[rekado]

I do not use Docker, so I cannot help with that, I'm afraid.

Guix can be installed by unpacking a tarball on any GNU/Linux system. Here's a short introduction: http://elephly.net/posts/2015-06-21-getting-started-with-guix.html

To build julia 0.3.6 as previously packaged: guix build julia
To build with the same recipe but another release tarball: guix build --with-source=thetarball.tar.gz julia
To create an environment containing all of julia's dependencies needed for building it manually: guix environment julia

[vtjnash]

looks like we could just stick a dummy NULL at the end of the mattrs array initializer to make gcc happy

[tkelman]

That sounds worth trying, @rekado can you test that and submit a PR with a patch if you come up with one that works?

[rekado]

Good idea. I'll try that today and report back.

[rekado]

After applying the following patch Julia 0.3.9 builds just fine:

--- a/src/codegen.cpp   2015-06-24 12:44:31.218674066 +0200
+++ b/src/codegen.cpp   2015-04-23 11:19:50.000000000 +0200
@@ -4551,7 +4551,7 @@
 #ifdef V128_BUG
         "-avx",
 #endif
-    };
+    ""};
     SmallVector<std::string, 4> MAttrs(mattr, mattr+sizeof(mattr)/sizeof(mattr[0]));
     EngineBuilder eb = EngineBuilder(engine_module)
         .setEngineKind(EngineKind::JIT)

Thank you, @vtjnash for the hint. A NULL won't work, because that would fail with

terminate called after throwing an instance of 'std::logic_error'
  what():  basic_string::_S_construct null not valid

as NULL is not a string.

[nalimilan]

@rekado Is LLVM happy when you pass it an empty string?

If so, would you prepare a pull request with your patch? Though I think you should only add the empty string when none of the #ifdefs apply. Also, a comment to explain what's going on would be nice.

[rekado]

@nalimilan I don't know how to detect unhappiness in LLVM, but I did not see it complain during the build.

I can prepare a nicer patch later and create a pull request (but probably not today).

[vtjnash]

llvm seems fine with skipping over "", so i think it's best to go with the simple option and just unconditionally add "" to the end of the list.

$ ./llc-3.3 -mattr=-avx,,+avx2,,,,"",'',,,
^D
      .section  __TEXT,__text,regular,pure_instructions

.subsections_via_symbols
$ ./llc-3.3 -mattr=-avx,,+avx2,,,,"",'',,,vfp
^D
'+vfp' is not a recognized feature for this target (ignoring feature)
'+vfp' is not a recognized feature for this target (ignoring feature)
'+vfp' is not a recognized feature for this target (ignoring feature)
    .section    __TEXT,__text,regular,pure_instructions

.subsections_via_symbols
$

[rekado]

FWIW, I'm using the above patch with Julia 0.3.10.

[tkelman]

We'll probably do one last tag on 0.3, so a PR against the release-0.3 branch would be good to run that against CI.