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)

polly_LLVM_LINK_LLVM_DYLIB_fix.patch (504 bytes) - added by howarth.at.macports@… 9 years ago.
back port of polly trunk r259659 to properly link LLVMPolly.so when LLVM_LINK_LLVM_DYLIB is enabled
fix-PR26393.patch (1.5 KB) - added by howarth.at.macports@… 9 years ago.
patch to prune static LLVM component lib linkages when LLVM_LINK_LLVM_DYLIB is enabled
Portfile.diff (2.1 KB) - added by howarth.at.macports@… 9 years ago.
switch build to use LLVM_LINK_LLVM_DYLIB and remove unnecessary openmp variant

Download all attachments as: .zip

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.8llvm-3.8: remove unnecessary openmp variant

comment:3 Changed 9 years ago by howarth.at.macports@…

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.

Changed 9 years ago by howarth.at.macports@…

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 
Last edited 9 years ago by howarth.at.macports@… (previous) (diff)

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: newclosed
Note: See TracTickets for help on using tickets.