[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
llvm-commits at lists.llvm.org
Tue May 4 11:02:08 PDT 2021
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
More information about the llvm-commits
mailing list