yaxunl added a comment. In D117137#3273330 <https://reviews.llvm.org/D117137#3273330>, @tra wrote:
> In D117137#3269365 <https://reviews.llvm.org/D117137#3269365>, @yaxunl wrote: > >> Does that mean only "spirv{64}-unknown-unknown" is acceptable, or >> "spirv{64}-amd-unknown-unknown" is also acceptable? > > My point is that `unknown` part of the triple is a catch-all for `anything, > including something invalid` and should not be used for positive checks. > If we do not care about those parts of the triple ( accepting invalid triple > would imply it), then we should not check those parts at all. > Otherwise it leads to a weird inconsistency -- invalid triple like > `spirv64-foo-bar`is accepted, but an equally nonsensical triple like > `spir64-suse-whatever`which happens to use a known vendor or OS parts is not. > > The bottom line is that if there's currently no known use of the > vendor/OS/env parts of the triple, then we should not be checking them. > If we do want to accept specific triple, then appropriate enums should be > used/added. I get your point. `TT.getVendor() == llvm::Triple::UnknownVendor` and `TT.getOS() == llvm::Triple::UnknownOS` checks the processed vendor/OS string instead of the original string, which could be misleading. Since SPIRV backend requires OS and environment to be unknown. It seems we'd better check the original OS and environment string in the Triple by splitting the triple by `-` and taking the 3rd and 4th element (https://github.com/llvm/llvm-project/blob/main/llvm/lib/Support/Triple.cpp#L795). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117137/new/ https://reviews.llvm.org/D117137 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
