Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
rekado opened this issue Jun 23, 2015 · 14 comments
Closed

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

rekado opened this issue Jun 23, 2015 · 14 comments

Comments

@rekado
Copy link

rekado commented Jun 23, 2015

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
Copy link
Contributor

tkelman commented Jun 23, 2015

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

@rekado
Copy link
Author

rekado commented Jun 23, 2015

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
Copy link
Contributor

tkelman commented Jun 23, 2015

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
Copy link
Author

rekado commented Jun 23, 2015

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

@JeffBezanson JeffBezanson changed the title build error: zero-size array ‘mattr’ build error: zero-size array ‘mattr’ with LLVM 3.5 Jun 23, 2015
@vtjnash
Copy link
Sponsor Member

vtjnash commented Jun 24, 2015

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

@tkelman
Copy link
Contributor

tkelman commented Jun 24, 2015

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
Copy link
Author

rekado commented Jun 24, 2015

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

@rekado
Copy link
Author

rekado commented Jun 24, 2015

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
Copy link
Member

@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
Copy link
Author

rekado commented Jun 24, 2015

@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
Copy link
Sponsor Member

vtjnash commented Jun 24, 2015

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
$

vtjnash added a commit that referenced this issue Jul 8, 2015
this seems likely to be a better default for the majority of users. plus the failure mode is a SIGILL, rather than random data getting returned from floating point computations
@rekado
Copy link
Author

rekado commented Jul 21, 2015

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

@tkelman
Copy link
Contributor

tkelman commented Jul 21, 2015

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.

@tkelman
Copy link
Contributor

tkelman commented Jul 21, 2015

closed by #12243

@tkelman tkelman closed this as completed Jul 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants