tra added a subscriber: linjamaki. tra added inline comments.
================ Comment at: clang/lib/Driver/Driver.cpp:153-155 + if (TT->getArch() == llvm::Triple::spirv64 && + TT->getVendor() == llvm::Triple::UnknownVendor && + TT->getOS() == llvm::Triple::UnknownOS) ---------------- dcastagna wrote: > dcastagna wrote: > > tra wrote: > > > What's expected to happen if someone specifies `spirv64-nvidia` ? > > > > > > If someone uses `spirv64-foo-bar` I think it would match this condition. > > > > > > Accepting `spirv64-foo-bar`, but not `spirv64-nvidia-unknown` would be > > > somewhat odd. I think we either should check a fully-specified triple, or > > > only check the parts that matter -- `getArch()` in this case, or, maybe, > > > arch and vendor. > > This part is the check for the hip offload triple and this patch did not > > change the logic for HIP (at least not intentionally), it should be the > > same as the logic specified in the current getHIPOffloadTargetTriple on ToT. > > Happy to change it if you think it shuold be different though. > Removed the vendor and os check to make it consistent with the cuda logic. @linjamaki, @yaxunl -- are you OK with ignoring the vendor/OS parts for spirv triples? 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
