tra added a comment.
In D88524#2304224 <https://reviews.llvm.org/D88524#2304224>, @yaxunl wrote:
> This translates to "unknown" in string form, which feels arbitrary. What if a
> target has a valid cpu name which "unknown"? Isn't a nullptr (empty string in
> string form) a more generic format to represent an invalid target?
It was the `nullptr` that got me digging. I was not sure what it would mean and
do. Generally speaking it's a 'magic' value with the meaning that's not at all
clear, even if it does what you want. I'd rather have a constant or enum with a
clear meaning.
> Is it OK to make the change HIP specific? i.e. let HIP toolchain use empty
> string for invalid target whereas CUDA toolchain keep using "unknown".
Putting magic value behind if(HIP) does not make it any better.
`TargetID(nullptr)` is the same as `TargetID("")` because StringRef(nullptr) is
the same as an empty string. I'd add a CudaArch::UNUSED and modify
`CudaVersionToString` to return "". That should make
`HIPToolChain::TranslateArgs()`skip it as it would for nullptr.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D88524/new/
https://reviews.llvm.org/D88524
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
