Opened 9 years ago
Closed 9 years ago
#50509 closed enhancement (fixed)
llvm-3.8: remove unnecessary openmp variant
Reported by: | howarth.at.macports@… | Owned by: | jeremyhu (Jeremy Huddleston Sequoia) |
---|---|---|---|
Priority: | Normal | Milestone: | |
Component: | ports | Version: | 2.3.4 |
Keywords: | haspatch | Cc: | |
Port: | llvm-3.8 |
Description
The attached Portfile diff eliminates the unnecessary openmp variant now that the 3.8 branch cfe sources default to -fopenmp=libomp for -fopenmp.
Attachments (3)
Change History (14)
comment:1 Changed 9 years ago by ryandesign (Ryan Carsten Schmidt)
Cc: | jeremyhu@… removed |
---|---|
Keywords: | haspatch added |
Owner: | changed from macports-tickets@… to jeremyhu@… |
comment:2 Changed 9 years ago by ryandesign (Ryan Carsten Schmidt)
Summary: | remove unnecessary openmp variant from llvm-3.8 → llvm-3.8: remove unnecessary openmp variant |
---|
comment:3 Changed 9 years ago by howarth.at.macports@…
Changed 9 years ago by howarth.at.macports@…
Attachment: | polly_LLVM_LINK_LLVM_DYLIB_fix.patch added |
---|
back port of polly trunk r259659 to properly link LLVMPolly.so when LLVM_LINK_LLVM_DYLIB is enabled
comment:4 Changed 9 years ago by jeremyhu (Jeremy Huddleston Sequoia)
I'm going to just wait for https://llvm.org/bugs/show_bug.cgi?id=26393 to land and then pull in the changes from upstream.
Changed 9 years ago by howarth.at.macports@…
Attachment: | fix-PR26393.patch added |
---|
patch to prune static LLVM component lib linkages when LLVM_LINK_LLVM_DYLIB is enabled
comment:5 Changed 9 years ago by howarth.at.macports@…
Confirmation of proper linkages can be seen in llvm-3.8 build with..
$ cat /Users/howarth/ports/lang/llvm-3.8/work/build/tools/opt/CMakeFiles/opt.dir/link.txt /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ -pipe -Os -std=c++11 -stdlib=libc++ -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -std=c++11 -g -arch x86_64 -mmacosx-version-min=10.11 -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/opt/local/lib -Wl,-headerpad_max_install_names -Wl,-rpath,@loader_path -rdynamic CMakeFiles/opt.dir/AnalysisWrappers.cpp.o CMakeFiles/opt.dir/BreakpointPrinter.cpp.o CMakeFiles/opt.dir/GraphPrinters.cpp.o CMakeFiles/opt.dir/NewPMDriver.cpp.o CMakeFiles/opt.dir/PassPrinters.cpp.o CMakeFiles/opt.dir/PrintSCC.cpp.o CMakeFiles/opt.dir/opt.cpp.o -o ../../bin/opt ../../lib/libLLVM.dylib -Wl,-rpath,@executable_path/../lib $ cat /Users/howarth/ports/lang/llvm-3.8/work/build/tools/llc/CMakeFiles/llc.dir/link.txt /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ -pipe -Os -std=c++11 -stdlib=libc++ -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -std=c++11 -g -arch x86_64 -mmacosx-version-min=10.11 -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/opt/local/lib -Wl,-headerpad_max_install_names -Wl,-rpath,@loader_path -rdynamic CMakeFiles/llc.dir/llc.cpp.o -o ../../bin/llc ../../lib/libLLVM.dylib -Wl,-rpath,@executable_path/../lib
and in clang-3.8 build with...
$ cat /Users/howarth/ports/lang/llvm-3.8/work/build/tools/clang/tools/driver/CMakeFiles/clang.dir/link.txt /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ -pipe -Os -std=c++11 -stdlib=libc++ -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -std=c++11 -fno-common -Woverloaded-virtual -Wno-nested-anon-types -g -arch x86_64 -mmacosx-version-min=10.11 -Wl,-search_paths_first -Wl,-headerpad_max_install_names -L/opt/local/lib -Wl,-headerpad_max_install_names -Wl,-rpath,@loader_path -rdynamic CMakeFiles/clang.dir/driver.cpp.o CMakeFiles/clang.dir/cc1_main.cpp.o CMakeFiles/clang.dir/cc1as_main.cpp.o -o ../../../../bin/clang-3.8 ../../../../lib/libLLVM.dylib ../../../../lib/libclangBasic.a ../../../../lib/libclangCodeGen.a ../../../../lib/libclangDriver.a ../../../../lib/libclangFrontend.a ../../../../lib/libclangFrontendTool.a -Wl,-sectcreate,__TEXT,__info_plist,/opt/local/var/macports/build/_Users_howarth_ports_lang_llvm-3.8/clang-3.8/work/build/tools/clang/tools/driver/Info.plist ../../../../lib/libclangCodeGen.a ../../../../lib/libclangRewriteFrontend.a ../../../../lib/libclangARCMigrate.a ../../../../lib/libclangStaticAnalyzerFrontend.a ../../../../lib/libclangFrontend.a ../../../../lib/libclangDriver.a ../../../../lib/libclangParse.a ../../../../lib/libclangSerialization.a ../../../../lib/libclangSema.a ../../../../lib/libclangEdit.a ../../../../lib/libclangStaticAnalyzerCheckers.a ../../../../lib/libclangStaticAnalyzerCore.a ../../../../lib/libclangAnalysis.a ../../../../lib/libclangAST.a ../../../../lib/libclangRewrite.a ../../../../lib/libclangLex.a ../../../../lib/libclangBasic.a -Wl,-rpath,@executable_path/../lib
comment:6 Changed 9 years ago by howarth.at.macports@…
Confirmation of proper linkage of LLVMPolly.so can seen in the llvm-3.8 build with...
cat /Users/howarth/ports/lang/llvm-3.8/work/build/tools/polly/lib/CMakeFiles/LLVMPolly.dir/link.txt /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/clang++ -pipe -Os -std=c++11 -stdlib=libc++ -fPIC -fvisibility-inlines-hidden -Wall -W -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wcovered-switch-default -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -std=c++11 -fno-exceptions -fno-rtti -g -arch x86_64 -mmacosx-version-min=10.11 -bundle -Wl,-headerpad_max_install_names -Wl,-flat_namespace -Wl,-undefined -Wl,suppress -L/opt/local/lib -Wl,-headerpad_max_install_names -Wl,-rpath,@loader_path -Wl,-flat_namespace -Wl,-undefined -Wl,suppress -o ../../../lib/LLVMPolly.so CMakeFiles/LLVMPolly.dir/Polly.cpp.o -L/opt/local/var/macports/build/_Users_howarth_ports_lang_llvm-3.8/llvm-3.8/work/build/./lib ../../../lib/libPolly.a ../../../lib/libPollyISL.a ../../../lib/libLLVM.dylib -Wl,-rpath,@executable_path/../lib
comment:7 Changed 9 years ago by howarth.at.macports@…
Note there will be a major speed improvement in the llvm tool chain with -DLLVM_LINK_LLVM_DYLIB:BOOL=ON compared to -DLLVM_BUILD_LLVM_DYLIB=ON as explained by Chris Bieneman...
All platforms see performance implications of dynamically linking libraries as opposed to statically linking them. I don’t know whether that hits Darwin or Linux worse, but I believe Darwin’s two-level name spacing actually makes Darwin better than Linux. If I understand correctly it hits windows worst of all because of how Windows handles weak symbol resolution.
On all platforms I would expect the many shared libraries to be way worse relative to one shared library. I expect that would be the case because one of the complications of C++ is a lot of weak exports. Those come from functions implemented in headers which are then compiled into each generated object file. At static link time the linker resolves the duplicates down to a single implementation. If you are linking one dynamic library you will have way less symbols than if you are linking multiple libraries because each library will have its own copy of the duplicated symbols.
comment:8 Changed 9 years ago by jeremyhu (Jeremy Huddleston Sequoia)
Yes, I am quite aware of the benefits. The issue isn't about the benefits of the change so much as the time involved and my tight schedule. I'm going to just wait for these changes to land upstream and pull them in the next time I bump the port.
comment:9 Changed 9 years ago by howarth.at.macports@…
The polly fix is now in both trunk and current 3.8 branch. I am trying to get Chris Bieneman to review and check the llvm linkage fix before 3.8 is released.
Changed 9 years ago by howarth.at.macports@…
Attachment: | Portfile.diff added |
---|
switch build to use LLVM_LINK_LLVM_DYLIB and remove unnecessary openmp variant
comment:10 Changed 9 years ago by howarth.at.macports@…
Note that the patch...
http://reviews.llvm.org/D16945
to fix https://llvm.org/bugs/show_bug.cgi?id=26393 is now back ported upstream at r260693 so no new patches are required for the -DLLVM_LINK_LLVM_DYLIB=ON build.
comment:11 Changed 9 years ago by jeremyhu (Jeremy Huddleston Sequoia)
Resolution: | → fixed |
---|---|
Status: | new → closed |
Switch build from -DLLVM_BUILD_LLVM_DYLIB=ON to -DLLVM_LINK_LLVM_DYLIB:BOOL=ON. Add polly_LLVM_LINK_LLVM_DYLIB_fix.patch (upstream LLVM r259659) to properly link LLVMPolly.so in polly variant against libLLVM.dylib. Add pending fix-PR26393.patch patch eliminate incorrect linkages of LLVM component static libs in addition to libLLVM.dylib.