[PATCH] D110622: [HIPSPV][3/4] Enable SPIR-V emission for HIP

Tue Dec 7 13:19:33 PST 2021

yaxunl added a comment.

In D110622#3177111 <https://reviews.llvm.org/D110622#3177111>, @tra wrote:

> In D110622#3176804 <https://reviews.llvm.org/D110622#3176804>, @yaxunl wrote:
>
>>>> So, the question is -- what's the right way to specify something like this in a consistent manner? 
>>>> `--offload` option proposed here does not seem to be a good fit. It was intended as a more flexible way to create a single `-cc1` sub-compilation and we're doing quite a bit more here.
>>>
>>> Does `--offload-arch=spirv*` fit better here? If I understand the goal of this patch correctly, it tries to provide controls for changing offload target for HIP application from default (AMDGCN) to SPIR-V.
>>
>> `--offload-arch=` only accepts GPU arch which is translated to processor option (-mcpu= or -march=) in clang -cc1. spirv is a target triple which is not suitable for `--offload-arch=`.
>>
>> `--offload=` is supposed to cover both target triple and processor with some flexibility. If only target triple is specified, it assumes default processor. If only processor is specified, it deduces target triple. It also allows both triple and processor. In this case, `--offload=spirv` translates to -triple spirv -mcpu=generic.
>
> So, one would expect that we should be able to specify it more than once to target multiple GPU variants, if we were to use it as a more flexible `--offload-arch`.
> If I read the tests correctly, using `--offload=` limits us to exactly one variant now. Perhaps it should eventually be relaxed to only enforce single `--offload=` variant if we're offloading to SPIR-V. It's not a showstopper for this patch. We can relax it later.

I don't think `--offload=` is restricted to be specified only once. The test checks `--offload-arch=` and `--offload=` are mutually exclusive.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110622