[PATCH] D156671: [AMDGPU][SIInsertWaitcnts] Initialize the WaitcntBrackets for non-kernel functions

[Juan Manuel Martinez Caamaño via Phabricator via llvm-commits]

jmmartinez added a comment.

In D156671#4546645 <https://reviews.llvm.org/D156671#4546645>, @foad wrote:

> In D156671#4546620 <https://reviews.llvm.org/D156671#4546620>, @jmmartinez wrote:
>
>> In D156671#4546409 <https://reviews.llvm.org/D156671#4546409>, @foad wrote:
>>
>>> Please explain what this is for. Is it a bug fix? Do you have a test that shows the intended effect?
>>
>> Hello,
>>
>> The test that best depicts the issue is amd.endpgm.ll <https://reviews.llvm.org/differential/changeset/?ref=4373291>.
>>
>> In this test there is a `test1` non-kernel function that inmediately does a call to `@llvm.amdgcn.endpgm`. On GFX11, I'd expect a `s_sendmsg` instruction before the `s_endpgm` for that function.
>>
>> `SIInsertWaitcnt` inserts an `s_sendmsg` before each `s_endpgm` if the `VS_CNT` score is not 0 and if there is any pending scratch store operation; but when `SIInsertWaitcnt` initializes the `WaitcntBrackets` object for any function (entry or not) all the counters are set to 0.
>
> Thanks for the explanation. A simpler implementation of this would just set VS_CNT to its max value, not all the counters.
>
> //However// you can only insert the `s_sendmsg sendmsg(MSG_DEALLOC_VGPRS)` if there are //**no**// pending scratch stores (see D153295 <https://reviews.llvm.org/D153295>) so I don't think this patch is acceptable.

Argh ! I miss-read the code. Sorry for that.

There is still something strange in how the `WaitcntBrackets` object is initialized that I find strange: It seems that the counters and the pending events are assumed to be 0 at the entry of non-kernel functions, which looks wrong to me. Am I right? Or I'm missinterpreting how `WaitcntBrackets` works (or how the calling convention works)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156671