Re: [PATCH] D25932: Unconditionally pass `-lto_library` to the linker on Darwin
I would also note that llvm 3.9.x and 4.0svn also require use of their own llvm-ar and llvm-ranlib for archiving under -flto as well (beyond using the matching libLTO.dylib). On Tue, Nov 22, 2016 at 12:59 AM, Mehdi Amini wrote: > Double-checked on the latest binary release on llvm.org, it ships with > clang+llvm-3.9.0-x86_64-apple-darwin/lib/libLTO.dylib > > I also can’t find any CMake option that disable LTO support at build time > for clang. > > > On Nov 21, 2016, at 9:53 PM, Mehdi Amini via cfe-commits > wrote: > > AFAIK any clang build open-source ships with libLTO. > Not having libLTO built with clang is a Chromium oddity, unless I missed the > obvious somewhere. > > > On Nov 21, 2016, at 9:50 PM, Nico Weber wrote: > > In what way is this chromium specific? It's "all non-xcode uses of clang on > mac", no? > > > On Nov 21, 2016 7:29 PM, "Mehdi Amini" wrote: >> >> >> On Nov 21, 2016, at 2:44 PM, Nico Weber wrote: >> >> On Mon, Nov 21, 2016 at 5:34 PM, Mehdi Amini >> wrote: >>> >>> >>> On Nov 21, 2016, at 2:27 PM, Nico Weber wrote: >>> >>> On Mon, Nov 21, 2016 at 5:19 PM, Mehdi AMINI via cfe-commits >>> wrote: mehdi_amini added a comment. In https://reviews.llvm.org/D25932#601842, @rnk wrote: > In https://reviews.llvm.org/D25932#601820, @mehdi_amini wrote: > > > We ship `clang + libLTO + ld64` bundled in the toolchain, so even if > > you don't package libLTO yourself, it is already accessible from the > > linker: > > it will use the one in the toolchain when needed. > > > > I don't have an immediate idea to prevent this and have the linker > > issue an error (other than removing manually libLTO from the Xcode > > installation). > > > So, even if clang doesn't pass -lto_library to ld64, ld64 will > auto-discover the bundled libLTO that happens to be next to it? That > could > go badly. Right: until LLVM 3.8, clang was *never* passing the `-lto_library` option. The only way to have your own libLTO used by ld64 instead of the one in the Xcode toolchain was setting the environment variable "DYLD_LIBRARY_PATH". Of course was has many issues, and that's what lead us to have clang passing this option to ld64. Initially only when the driver was invoked with -flto, but recently I had issues with clients that didn't use LTO themselves but were having static archives they depend on that were containing bitcode. >>> >>> >>> Where those archives system libraries, or other things? >>> >>> >>> We have two cases: >>> >>> 1) Internal teams producing libraries in an internal SDK with LTO >>> enabled, and other teams consuming these libraries and linking to the >>> framework. It seems also something that people out-in-the-wild are doing >>> according to some bug reports. >>> 2) Any Xcode user that have a somehow complex project which is split in >>> multiple targets. Xcode can’t tell clang from one target that it is linking >>> with LTO even if LTO is disabled just because another dependency has LTO >>> enabled. And sometimes Xcode is only seeing static archive as an input >>> anyway. >> >> >> It sounds like this is a pure regression for us then. >> >> >> Right, for you "downstream consumer of clang in chromium”. >> >> Since 'it never "hurt" to pass it' isn't true (every link invocation done >> by the driver now prints a warning), maybe this should be reverted until >> there's some better approach? >> >> Requiring everyone to put a dummy libLTO.dylib at ../lib/libLTO.dylib >> (while clever) seems pretty unfortunate. >> >> >> Is there a CMake invocation that disable libLTO today and allow to run >> “make install” and produce a distribution of clang without libLTO? >> >> If not, then I’m against reverting this because I consider your Chromium >> specific incorrect with respect to the support upstream. And I’m fine having >> it supported in the future, but you should make it supported, for instance >> with a cmake option (if the cmake option already exists, I haven’t checked, >> then we could conditionally compile-out the warning based on it). >> >> — >> Mehdi >> >> >> >> >> >>> >>> >>> Maybe clang could sniff archives for bitcode and pass only -flto in that >>> case? >>> >>> >>> That seems like a possibility. It’d have to resolve paths to the static >>> archives, which it doesn’t do right now I believe (they can be resolved with >>> `-Lpath -lfoo`). >>> >>> — >>> Mehdi >> >> > > ___ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits > > ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25932: Unconditionally pass `-lto_library` to the linker on Darwin
jwhowarth reopened this revision. jwhowarth added a comment. This approach doesn't work if the user installs clang in a buried subdirectory such as /sw/opt/llvm-4.0 but accesses the compilers via a /sw/bin/clang-4.0 symlink pointing at /sw/opt/llvm-4.0/bin/clang-4.0... $ /sw/bin/clang-4.0 -flto himenoBMTxpa.c clang-4.0: warning: libLTO.dylib relative to clang installed dir not found; using 'ld' default search path instead [-Wliblto] ... "/usr/bin/ld" -demangle -object_path_lto /var/folders/vh/xthx1f251nqfj804049zl1wmgn/T/cc-083c59.o -no_deduplicate -dynamic -arch x86_64 -macosx_version_min 10.11.0 -o a.out /var/folders/vh/xthx1f251nqfj804049zl1wmgn/T/himenoBMTxpa-34b8ea.o -lSystem /sw/opt/llvm-4.0/bin/../lib/clang/4.0.0/lib/darwin/libclang_rt.osx.a $ /sw/opt/llvm-4.0/bin/clang-4.0 -flto himenoBMTxpa.c -v ... "/usr/bin/ld" -demangle -object_path_lto /var/folders/vh/xthx1f251nqfj804049zl1wmgn/T/cc-afbbef.o -lto_library /sw/opt/llvm-4.0/lib/libLTO.dylib -no_deduplicate -dynamic -arch x86_64 -macosx_version_min 10.11.0 -o a.out /var/folders/vh/xthx1f251nqfj804049zl1wmgn/T/himenoBMTxpa-44b09e.o -lSystem /sw/opt/llvm-4.0/bin/../lib/clang/4.0.0/lib/darwin/libclang_rt.osx.a Repository: rL LLVM https://reviews.llvm.org/D25932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25932: Unconditionally pass `-lto_library` to the linker on Darwin
jwhowarth added a comment. Opened https://llvm.org/bugs/show_bug.cgi?id=30811 since the inability for symlinks to be handled in locating libLTO.dylib defeats the purpose of this commit. Repository: rL LLVM https://reviews.llvm.org/D25932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D25932: Unconditionally pass `-lto_library` to the linker on Darwin
jwhowarth added a comment. In https://reviews.llvm.org/D25932#581167, @mehdi_amini wrote: > I just verified that I reproduce with -flto and an previous clang version. This issue will only be triggered if you build with "-DCMAKE_INSTALL_PREFIX:PATH=/sw/opt/llvm-4.0" to place llvm in a buried subdirectory and access the compilers with symlinks in /sw/bin pointing at the actual compilers in /sw/opt/llvm-4.0/bin. The call to llvm::sys::path::parent_path(D.getInstalledDir()) incorrectly places the install level at /sw instead of the actual sw/opt/llvm-4.0. Repository: rL LLVM https://reviews.llvm.org/D25932 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13802: [OPENMP] Make -fopenmp to turn on OpenMP support by default.
jhowarth added a subscriber: jhowarth. jhowarth added a comment. The proposed patches here to change the default behavior of -fopenmp from -fopenmp=libgomp to -fopenmp=libomp seem to only handle the configure-based build. The following change required to switch over the crake-based build to default -fopenmp to libomp is missing. Index: CMakeLists.txt === --- CMakeLists.txt(revision 251539) +++ CMakeLists.txt(working copy) @@ -196,7 +196,7 @@ set(GCC_INSTALL_PREFIX "" CACHE PATH "Di set(DEFAULT_SYSROOT "" CACHE PATH "Default to all compiler invocations for --sysroot=." ) -set(CLANG_DEFAULT_OPENMP_RUNTIME "libgomp" CACHE STRING +set(CLANG_DEFAULT_OPENMP_RUNTIME "libomp" CACHE STRING "Default OpenMP runtime used by -fopenmp.") set(CLANG_VENDOR "" CACHE STRING http://reviews.llvm.org/D13802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D13802: [OPENMP] Make -fopenmp to turn on OpenMP support by default.
jhowarth added a comment. The CMakeLists.txt change cited is in top-level of the clang sources and not in the llvm top-level. http://reviews.llvm.org/D13802 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: r258307 - [OPENMP 4.0] Fix for codegen of 'cancel' directive within 'sections' directive.
Tested the attached patch which contains a back port of the net
changes from both r258307 and
Author: abataev
Date: Fri Jan 22 02:56:50 2016
New Revision: 258495
URL: http://llvm.org/viewvc/llvm-project?rev=258495&view=rev
Log:
[OPENMP] Generalize codegen for 'sections'-based directive.
If 'sections' directive has only one sub-section, the code for
'single'-based directive was emitted. Removed this codegen, because it
causes crashes in different cases.
Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/test/OpenMP/cancel_codegen.cpp
cfe/trunk/test/OpenMP/cancellation_point_codegen.cpp
cfe/trunk/test/OpenMP/parallel_sections_codegen.cpp
cfe/trunk/test/OpenMP/sections_codegen.cpp
cfe/trunk/test/OpenMP/sections_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/sections_lastprivate_codegen.cpp
cfe/trunk/test/OpenMP/sections_private_codegen.cpp
cfe/trunk/test/OpenMP/sections_reduction_codegen.cpp
on x86_64-apple-darwin15 without regressions in the cfe or libomp test suites.
Jack
ps Also verified that no regressions occur in the OpenMP3.1_Validation
test suite.
On Tue, Jan 26, 2016 at 2:23 PM, Hans Wennborg wrote:
> Did that fix land, and should it be merged to 3.8?
>
> On Thu, Jan 21, 2016 at 7:03 PM, Alexey Bataev wrote:
>> Later today I will post another fix, that will fix all 'sections'
>> related troubles, including this one. So I don't think it is necessary
>> to merge it
>>
>> Best regards,
>> Alexey Bataev
>> =
>> Software Engineer
>> Intel Compiler Team
>>
>> 22.01.2016 0:10, Hans Wennborg пишет:
>>> Jack suggested (https://llvm.org/bugs/show_bug.cgi?id=26059#c7) that
>>> this should be merged to 3.8.
>>>
>>> Alexey, you're the code owner here. OK for merging? If yes, do you
>>> want to go ahead and merge with utils/release/merge.sh?
>>>
>>> On Wed, Jan 20, 2016 at 4:29 AM, Alexey Bataev via cfe-commits
>>> wrote:
Author: abataev
Date: Wed Jan 20 06:29:47 2016
New Revision: 258307
URL: http://llvm.org/viewvc/llvm-project?rev=258307&view=rev
Log:
[OPENMP 4.0] Fix for codegen of 'cancel' directive within 'sections'
directive.
Allow to emit code for 'cancel' directive within 'sections' directive with
single sub-section.
Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/test/OpenMP/cancel_codegen.cpp
Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=258307&r1=258306&r2=258307&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Wed Jan 20 06:29:47 2016
@@ -3685,8 +3685,6 @@ void CGOpenMPRuntime::emitCancelCall(Cod
// kmp_int32 cncl_kind);
if (auto *OMPRegionInfo =
dyn_cast_or_null(CGF.CapturedStmtInfo)) {
-if (OMPRegionInfo->getDirectiveKind() == OMPD_single)
- return;
auto &&ThenGen = [this, Loc, CancelRegion,
OMPRegionInfo](CodeGenFunction &CGF) {
llvm::Value *Args[] = {
Modified: cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
URL:
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp?rev=258307&r1=258306&r2=258307&view=diff
==
--- cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp Wed Jan 20 06:29:47 2016
@@ -1786,7 +1786,11 @@ CodeGenFunction::EmitSections(const OMPE
CGF.EmitOMPPrivateClause(S, SingleScope);
(void)SingleScope.Privatize();
+auto Exit = CGF.getJumpDestInCurrentScope("omp.sections.exit");
+CGF.BreakContinueStack.push_back(BreakContinue(Exit, Exit));
CGF.EmitStmt(Stmt);
+CGF.EmitBlock(Exit.getBlock());
+CGF.BreakContinueStack.pop_back();
};
CGM.getOpenMPRuntime().emitSingleRegion(*this, CodeGen,
S.getLocStart(),
llvm::None, llvm::None,
llvm::None,
@@ -2647,7 +2651,8 @@ CodeGenFunction::getOMPCancelDestination
if (Kind == OMPD_parallel || Kind == OMPD_task)
return ReturnBlock;
assert(Kind == OMPD_for || Kind == OMPD_section || Kind ==
OMPD_sections ||
- Kind == OMPD_parallel_sections || Kind == OMPD_parallel_for);
+ Kind == OMPD_parallel_sections || Kind == OMPD_parallel_for ||
+ Kind == OMPD_single);
return BreakContinueStack.back().BreakBlock;
}
Modified: cfe/trunk/test/OpenMP/cancel_codegen.cpp
URL:
http://llvm.org/viewvc/ll
