[PATCH] D101681: [Compiler-rt] Distinguish between testing just built runtime libraries and the libraries shipped with the compiler.

[Dan Liew via Phabricator via llvm-commits]

delcypher added a subscriber: arphaman.
delcypher added inline comments.


================
Comment at: compiler-rt/test/lit.common.cfg.py:54
+    path = os.path.join(path, 'darwin')
+  else:
+    return None
----------------
phosek wrote:
> delcypher wrote:
> > delcypher wrote:
> > > @phosek I'd be curious how you think we should handle other platforms. If ` LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` wasn't a thing for Linux this would be just be something like.
> > > 
> > > ```lang=python
> > > if config.host_os == 'Linux':
> > >   path = os.path.join(path, 'linux')
> > > ```
> > > 
> > > but `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` makes this more complicated.
> > Hmm so doing something like `clang --target <target_triple> -print-runtime-dir` would actually help here. The problem is.
> > 
> > 1. This flag is so new it doesn't exist in the compilers I'm currently trying to test.
> > 2. It actually returns a completely incorrect path on Darwin. I've put up a patch for this https://reviews.llvm.org/D101682
> > 
> > I suppose I could have the current code as a fallback if `-print-runtime-dir` isn't supported, but otherwise use `-print-runtime-dir`.
> I'd prefer to make `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` the default, and eventually the only mode. It's something we've already discussed with @ldionne and I'm planning on sending out an RFC proposing that. Deprecating non-`LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` build might take a while so we may have to support both unfortunately for the foreseeable future.
As long as `-print-runtime-dir` continues to report the correct path when `LLVM_ENABLE_PER_TARGET_RUNTIME_DIR` is on then I think the approach used in this patch should be okay.

I'm expecting we can remove the fallback for older Apple Clang in this patch in ~ a year from now.

Please CC me and @arphaman on that RFC when you send it. As long as the new path doesn't contain the architecture on Apple platforms then I guess the new path would be okay.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D101681/new/

https://reviews.llvm.org/D101681