tahonermann wrote: I'm suggesting a couple of things:
1. The `SPIRV` enumerator is misnamed because it is strongly associated with AMDGCN; see `OffloadArchToString()` and `StringToOffloadArch()` in `clang/lib/Basic/OffloadArch.cpp` which maps (only) "amdgcnspirv" to `SPIRV` and would lose important information if `SPIRV` were to be interpreted as something other than AMDGCN/SPIR-V. 2. The changes indicate a common need to ask `A.isAMDGPU() || A.isSPIRV()` (or its negation). Every one of the replaced uses of `IsAMDOffloadArch()` follows this pattern. I appreciate that there may be cases in the future that will want to separately consider these architectures, but that seems unlikely to make this pattern disappear. It seems likely that there (will continue to be) a need to ask `isAMDGPU()`, `isAMDGCN_SPIRV()`, and `isAMDGPU_OR_AMDGCN_SPIRV()` (with a better name for the latter case). For the first case above, I very much want to see the `SPIRV` cases renamed to reflect their strong association with `AMDGCN` as was the case prior to https://github.com/llvm/llvm-project/pull/213362. I don't have a strong opinion on the second case, but it seems clear from the current pattern of uses that a helper function is warranted. https://github.com/llvm/llvm-project/pull/213363 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
