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

llvm 3.9: Download and build Polly #20941

Closed
wants to merge 16 commits into from
Closed

Conversation

spacekitteh
Copy link
Contributor

@spacekitteh spacekitteh commented Dec 6, 2016

Motivation for this change

#20890

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

@mention-bot
Copy link

@spacekitteh, thanks for your PR! By analyzing the history of the files in this pull request, we identified @gebner, @vcunat and @acowley to be potential reviewers.

@spacekitteh
Copy link
Contributor Author

spacekitteh commented Dec 6, 2016

Well, nox-review is going to take forever to check and build...

@spacekitteh spacekitteh changed the title [wip] llvm 3.9: Download and build Polly llvm 3.9: Download and build Polly Dec 6, 2016
@spacekitteh
Copy link
Contributor Author

I think this should probably have the mass-rebuild tag

@spacekitteh
Copy link
Contributor Author

Well, it looks like the travis instances don't have enough RAM to build it.

@spacekitteh
Copy link
Contributor Author

spacekitteh commented Dec 6, 2016

So, I get this error:

CMakeFiles/clang.dir/cc1_main.cpp.o: 
In function `cc1_main(llvm::ArrayRef<char const*>, char const*, void*)': cc1_main.cpp:
(.text._Z8cc1_mainN4llvm8ArrayRefIPKcEES2_Pv+0x35b): undefined reference to `polly::initializePollyPasses(llvm::PassRegistry&)'
collect2: error: ld returned 1 exit status

From what it appears, polly appears to be built in llvm.nix, but then it doesn't appear to be passed onto clang.nix?

I think clang may need to be patched to link the previously-created-in-llvm.nix polly.so that lives in the nix store, to avoid having to build polly a second time.

@spacekitteh
Copy link
Contributor Author

If I can't figure out how to appropriately patch the CMakeLists.txt, I don't think I can get around building polly twice: once in llvm.nix, and again in clang.nix.

I think this might be an example of the perfect use-case of multiple build outputs. Polly has to be in the llvm/tools/polly dir upon building clang in order to statically link it, unless the cmake file is patched.

@@ -26,7 +28,7 @@ let

postPatch = ''
sed -i -e 's/Args.hasArg(options::OPT_nostdlibinc)/true/' lib/Driver/Tools.cpp
sed -i -e 's/DriverArgs.hasArg(options::OPT_nostdlibinc)/true/' lib/Driver/ToolChains.cpp
sed -i -e 's/DriverArgs.hasArg(options::OPT_nostdlibinc)/true/' lib/Driver/ToolChains.cpp
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whitespace!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

death to whitespace

@copumpkin
Copy link
Member

@shlevy has also worked a fair amount on LLVM, if I remember correctly

@spacekitteh
Copy link
Contributor Author

spacekitteh commented Dec 6, 2016

So, I currently get the following error when using clang: : CommandLine Error: Option 'aarch64-ccmp' registered more than once! LLVM ERROR: inconsistency in registered CommandLine options

Possibly related: https://llvm.org/bugs/show_bug.cgi?id=22952

It might be because it's linking against a .so instead of a .a?

@spacekitteh
Copy link
Contributor Author

spacekitteh commented Dec 6, 2016

It looks like LLVM_BUILD_LLVM_DYLIB is broken, actually. I don't seem to have a libLLVM.so in my build output.

@spacekitteh
Copy link
Contributor Author

Could it be to do with having two separate builds?

] ++
# (stdenv.lib.optional llvm.enableSharedLibraries "-DLLVM_LINK_LLVM_DYLIB:Bool=true") ++
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how can i do this? That is, how to reference the input argument to a build dependency?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

eh, doesn't matter anyway. it doesn't fix the problem.

@spacekitteh
Copy link
Contributor Author

spacekitteh commented Dec 6, 2016

There's this message, too, from clang build:

WARNING: substitution variables should be valid bash names,
"ccLDFlags+" isn't and therefore was skipped; 
it might be caused by multi-line phases in variables - see #14907 for details.

@spacekitteh
Copy link
Contributor Author

spacekitteh commented Dec 7, 2016

I think it's a problem in this bit of code.

It thinks that in cc-wrapper ccCFlags+= is a variable being assigned called ccCFlags+, as opposed to the variableccCFlags being updated.

@spacekitteh spacekitteh force-pushed the patch-19 branch 2 times, most recently from 289b454 to ddb1321 Compare December 7, 2016 04:52
@spacekitteh
Copy link
Contributor Author

spacekitteh commented Dec 7, 2016

Damn, ddb13213531a539e616deb71afb915fe97f9ba22 didn't fix it. Anyone have any ideas? It probably helped with something, though, as I no longer got those #14907 messages.

@spacekitteh spacekitteh force-pushed the patch-19 branch 2 times, most recently from 056d7e5 to 9e9f8cc Compare December 7, 2016 08:49
@dtzWill
Copy link
Member

dtzWill commented Feb 19, 2017

By the way, did the problems motivating 31bdc2f ever get reported/fixed in Nixpkgs master?

Also--what's the status of this? Are there outstanding problems with this
(other than probably needing a squash/rebase to cleanup the history)?

@vcunat
Copy link
Member

vcunat commented Feb 20, 2017

I don't think it's been fixed, and I've never seen that problem causing anything more than warnings.

@Mic92
Copy link
Member

Mic92 commented Nov 15, 2018

This needs to be rewritten for newer llvm expressions

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

Successfully merging this pull request may close these issues.

None yet

6 participants