yaxunl added a comment. In D145770#4185658 <https://reviews.llvm.org/D145770#4185658>, @lamb-j wrote:
> In D145770#4184996 <https://reviews.llvm.org/D145770#4184996>, @yaxunl wrote: > >> The description needs fix. "ABI field" should be "environment component". >> >> Also, we need a clang-offload-bundler test which bundles with a >> non-canonical triple and unbundles with a canonical triple and vice versa. > > I'm not sure if we can use Triple.normalize(). Here's what I got from a small > test: > > amdgcn-amd-amdhsa -> amdgcn-amd-amdhsa (no env field added) > amdgcn-amd-amdhsa- -> amdgcn-amd-amdhsa-unknown (empty env changed to > unknown) > > Also are we sure ABI is incorrect? That's what is used here: > > https://clang.llvm.org/docs/CrossCompilation.html > > (I'm happy to change to environment, just double checking) https://github.com/llvm/llvm-project/blob/main/llvm/include/llvm/TargetParser/Triple.h only talks about "environment", which seems to be more accurate and up-to-date. It is unfortunate that we have used our own normalization (empty string for unspecified environment instead of "unknown") for triple. Looks like we have to keep that for now. Ideally, HIP runtime should accept "amdgcn-amd-amdhsa-unknown" in addition to "amdgcn-amd-amdhsa--" in fat binary, then we could switch to the LLVM Triple normalization. But that requires runtime change first. Probably we could do that in future. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D145770/new/ https://reviews.llvm.org/D145770 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
