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

Wed Sep 30 11:16:39 PDT 2020

yaxunl added a comment.

In D88524#2304173 <https://reviews.llvm.org/D88524#2304173>, @tra wrote:

>> Currently CUDA/HIP toolchain uses "unknown" as bound arch
>> for offload action for fat binary. This causes -mcpu or -march
>> with "unknown" added in HIPToolChain::TranslateArgs or
>> CUDAToolChain::TranslateArgs.
>
> It would appear that the problem is actually where we check TargetID -- we should've ignored CudaArch::UNKNOWN there.
> Not setting the arch here avoids triggering the bug but it does not fix it.
> Considering that `CudaArch::UNKNOWN` is used here to indicate that the arch is unused, perhaps we need an enum for that to distinguish it from unknown/unset.

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?

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".

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88524/new/

https://reviews.llvm.org/D88524