[Openmp-commits] [PATCH] D30015: Add arch-specific directory to search path

[Pirama Arumuga Nainar via Phabricator via Openmp-commits]

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 every binary linked by clang. We should only add rpath when we know the user is linking against a shared library provided by clang. See for example the way the darwin toolchain handles this in Toolchains.cpp.
> 
> Right now most libraries provided by clang are static, so there hasn't been much need to add rpath, but if we add more shared libraries (xray, sanitizers, profiling, etc), then I think we'll want to do this along with adding an option to suppress the implicit addition of rpath.
PTAL.


================
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/resource_dir
----------------
pirama wrote:
> mgorny wrote:
> > pirama wrote:
> > > Hahnfeld wrote:
> > > > Can you split that into two lines? Then it won't fail if there is some argument added in between
> > > Splitting into two lines makes FileCheck eagerly match the arch-subdir and -rpath into the {{.*}} next to the "-L".  This causes the check for -rpath to fail.
> > > 
> > > The test accepts intermediate arguments because of the wildcard right after the -L...Inputs/...
> > Please do not rely on implicit assumptions like that. One day someone may decide to 'fix' the wildcard not to match whitespace, and make the wrong assumption about order. If for anything, splitting in two would make this more readable.
> Do you have any suggestions?  Right now, if I split it into two lines, the second check fails due to the eager regex match.
Done.  I was able to get the exact path from the -cc1 invocation and use it to remove the wildcards.


https://reviews.llvm.org/D30015