[PATCH] D89980: [hip] Remove kernel argument coercion.

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D89980#2371270 <https://reviews.llvm.org/D89980#2371270>, @hliao wrote:

> In D89980#2368506 <https://reviews.llvm.org/D89980#2368506>, @arsenm wrote:
>
>> I think this is a dead end approach. I don't see the connection to the original problem you are trying to solve. Can you send me an IR testcase that this is supposed to help?
>
> That's probably commonly known. If we pass an aggregate parameter directly by value and dynamically index it late, that `alloca` cannot be promoted as that aggregate value in LLVM IR cannot be dynamically indexed. For example,
>
>   struct S {
>      int a[100];
>      int n;
>   };
>   
>   int foo(S s) {
>     return s.a[s.n];
>   }

This example is not a kernel

> If the underlying ABI chooses to pass `s` directly by value, we have the following pseudo IR.
>
>   %s = alloca S
>   ; store `s` value into %s as the parameter is treated as a local variable by filling its initial value from LLVM IR parameter.
>   ...
>   ; regular parameter access through %s with dynamic indices
>
> that `store` from the parameter from LLVM IR is an aggregate value store. Later, when %s is to be promoted, as it's once dynamically indexed, we cannot promote it as dynamic index on aggregate values is not representable in LLVM IR.
>
> In contrast, if a parameter is passed by value indirectly, that `store` is replaced with a `memcpy`. It's straightforward to promote '%s' as they are all memory operands of the same layout.
>
> If you need detailed IR, I may post here for your reference.

I need an actual source and IR example.  I think you are describing the missing promotion of pointers inside byref arguments. We need better promotion here, not eliminate it. It needs to cast the byref pointer, or cast the pointers inside the struct when accessed


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89980