[PATCH] D89077: [AMDGPU] Run hazard recognizer pass later

[Jay Foad via Phabricator via llvm-commits]

foad added a comment.

In D89077#2320804 <https://reviews.llvm.org/D89077#2320804>, @rampitec wrote:

> In D89077#2320691 <https://reviews.llvm.org/D89077#2320691>, @t-tye wrote:
>
>> In D89077#2320512 <https://reviews.llvm.org/D89077#2320512>, @kerbowa wrote:
>>
>>> In D89077#2320507 <https://reviews.llvm.org/D89077#2320507>, @t-tye wrote:
>>>
>>>> Is this now running after the waitcnt insertion pass? That would avoid the NOPs currently being inserted to split memory clauses that are not necessary as the waitcnt instructions will split the clauses.
>>>
>>> We also insert nops in the post-RA scheduler.
>>
>> In earlier conversation it was suggested that the spurious NOPs were explained as happening because the hazard recognizer inserted them to break memory clauses, and then the waitcnt pass ran. There would be no need to insert the NOPs if the waitcnt instructions were already there. So seems that was not a valid explanation, perhaps the post-RA scheduler is an explanation, but I am unclear why it put those ones in. @rampitec can you help explain?
>
> Post-RA scheduler and hazard recognizer is the same pass if you run post-RA scheduler. If not it is a separate pass.

Just to expand on this, GCNHazardRecognizer is not a pass, it's a class that gets plugged into two different passes:

1. The post-RA scheduler (which runs before waitcnt insertion)
2. The standalone hazard recognizer pass (which should be run pretty late, after any other pass that could introduce hazards)

The post-RA scheduler can insert NOPs which turn out to be unnecessary because a waitcnt will be inserted there anyway, which would fix the hazard. I reckon the right way to fix this is to teach GCNHazardRecognizer to return Hazard (an "advisory" hazard) instead of NoopHazard (a "mandatory" hazard) when it is called from the post-RA scheduler. That way, the post-RA scheduler gets to avoid some hazards by reordering instructions (but not inserting nops), and the rest of the hazards get fixed in the late hazard recognizer pass by inserting nops. I have tried to implement this in the past but never quite finished it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D89077