[PATCH] D101430: [AMDGPU] Refactor hazard recognition IsHazardFn and IsExpiredFn
Carl Ritson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 28 01:33:11 PDT 2021
critson added a comment.
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;
};
>> Looking at all the IsExpired implementations, none of them use this functionality (or at least not correctly).
>
> But I think they do? Notably the one in `getWaitStatesSince(IsHazardFn IsHazard, int Limit)`:
>
> auto IsExpiredFn = [Limit] (MachineInstr *, int WaitStates) {
> return WaitStates >= Limit;
> };
>
> But also the one in checkFPAtomicToDenormModeHazard.
Yes, I wrote the comment before I noticed all the small ones potentially using the early exit while refactoring.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101430/new/
https://reviews.llvm.org/D101430
More information about the llvm-commits
mailing list