[PATCH] D88524: [CUDA][HIP] Fix bound arch for offload action for fat binary

[Artem Belevich via Phabricator via cfe-commits]

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