[PATCH] D101430: [AMDGPU] Refactor hazard recognition IsHazardFn and IsExpiredFn

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

In D101430#2721933 <https://reviews.llvm.org/D101430#2721933>, @critson wrote:

> In D101430#2721920 <https://reviews.llvm.org/D101430#2721920>, @foad wrote:
>
>>> I do not think it is broken per-say but definitely confusing.
>>
>> I think it is broken because we are looping over predecessors trying to find the most recent hazard. The early-out allows the search to terminate if the hazard in some predecessor was too long ago, but then we would miss the fact that another predecessor might have a more recent hazard.
>
> I think what I meant was that if the caller required the closest hazard then it needs to supply IsExpired that ignores the nullptr probe, e.g.
>
>   auto IsExpiredFn = [Limit] (MachineInstr *MI, int WaitStates) {
>     return MI && WaitStates >= Limit;
>   };
>    

Surely all users always want the closest (most recent) hazard? I can't imagine the use case for anything else.

I still hope there is a way to fix the early-out case, rather than removing it completely, but I have not worked through the details yet.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101430