pirama added a comment.
Committed. Thanks for the reviews!
Repository:
rL LLVM
https://reviews.llvm.org/D30015
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL296927: Add arch-specific directory to search path (authored
by pirama).
Changed prior to commit:
https://reviews.llvm.org/D30015?vs=90532&id=90545#toc
Repository:
rL LLVM
https://reviews.llvm.org/D
pirama marked an inline comment as done.
pirama added inline comments.
Comment at: lib/Driver/Tools.cpp:2007-2009
+ // In the cross-compilation case, arch-specific library path is likely
+ // unavailable at runtime.
+ if (TC.isCrossCompiling()) return;
rnk wro
pirama updated this revision to Diff 90532.
pirama edited the summary of this revision.
pirama added a comment.
Fixed comment in test and added a test for -fsanitize=address without
-shared-libasan.
https://reviews.llvm.org/D30015
Files:
include/clang/Driver/ToolChain.h
lib/Driver/ToolChai
rnk accepted this revision.
rnk added a comment.
Looks good with a minor comment about a comment in the test case.
Comment at: lib/Driver/Tools.cpp:2007-2009
+ // In the cross-compilation case, arch-specific library path is likely
+ // unavailable at runtime.
+ if (TC.isCros
pirama updated this revision to Diff 90373.
pirama added a comment.
Add -rpath only when a runtime is included as a shared library. And, cleanup
tests.
https://reviews.llvm.org/D30015
Files:
include/clang/Driver/ToolChain.h
lib/Driver/ToolChain.cpp
lib/Driver/Tools.cpp
test/Driver/Inp
pirama marked 2 inline comments as done.
pirama added inline comments.
Comment at: lib/Driver/Tools.cpp:289
+ CmdArgs.push_back("-rpath");
+ CmdArgs.push_back(Args.MakeArgString(CandidateRPath.c_str()));
+}
rnk wrote:
> We shouldn't add rpath to eve
rnk added inline comments.
Comment at: include/clang/Driver/ToolChain.h:305
+ // as OpenMP) to find arch-specific libraries.
+ const std::string getArchSpecificLibPath() const;
+
Why const qualify the std::string?
Comment at: lib/Driver/Tools
pirama added a reviewer: rnk.
pirama added a subscriber: rnk.
pirama added a comment.
@rnk: This patch was to address discussion in
http://lists.llvm.org/pipermail/openmp-dev/2017-February/001659.html but now
also addresses
http://lists.llvm.org/pipermail/cfe-dev/2017-January/052512.html. Can
pirama added inline comments.
Comment at: test/Driver/arch-specific-libdir-rpath.c:18
+//
+// CHECK-ARCHDIR:
-L{{.*}}/Inputs/resource_dir_with_arch_subdir/lib/linux{{.*}} "-rpath"
{{.*}}/Inputs/resource_dir_with_arch_subdir/lib/linux
+// CHECK-NO-ARCHDIR-NOT: -L{{.*}}Inputs/res
mgorny accepted this revision.
mgorny added a comment.
LGTM modulo the test match split. But please wait for someone who has been
longer here to confirm.
Comment at: test/Driver/arch-specific-libdir-rpath.c:18
+//
+// CHECK-ARCHDIR:
-L{{.*}}/Inputs/resource_dir_with_arch_subd
pirama added a comment.
I believe I've addressed all open issues, but this patch has changed a lot
since Dan marked it accepted. @Hahnfeld or @mgorny: Can one of you give a
LGTM if you are satisfied?
https://reviews.llvm.org/D30015
___
cfe-commi
pirama added inline comments.
Comment at: test/Driver/arch-specific-libdir-rpath.c:18
+//
+// CHECK-ARCHDIR:
-L{{.*}}/Inputs/resource_dir_with_arch_subdir/lib/linux{{.*}} "-rpath"
{{.*}}/Inputs/resource_dir_with_arch_subdir/lib/linux
+// CHECK-NO-ARCHDIR-NOT: -L{{.*}}Inputs/res
mgorny added inline comments.
Comment at: lib/Driver/Tools.cpp:3267
+ if (llvm::sys::fs::is_directory(CandidateLibPath))
+CmdArgs.push_back(Args.MakeArgString("-L" + CandidateLibPath));
+
pirama wrote:
> mgorny wrote:
> > Don't you also need rpath for it? Or
pirama updated this revision to Diff 88865.
pirama marked an inline comment as done.
pirama added a comment.
Add arch name to -rpath test.
https://reviews.llvm.org/D30015
Files:
include/clang/Driver/ToolChain.h
lib/Driver/ToolChain.cpp
lib/Driver/Tools.cpp
test/Driver/Inputs/resource_di
15 matches
Mail list logo