This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Fix incorrect hazard mitigation
ClosedPublic

Authored by stepthomas on Jul 4 2023, 5:50 AM.

Details

Summary

GCNHazardRecognizer::fixVcmpxExecWARHazard() mitigates a specific hazard
by inserting a wait on sa_sdst==0 if such a wait isn't already present.
Unfortunately, the check for an existing wait incorrectly checks for one
that doesn't actually care about sa_sdst itself, but requires that no
other counters are waited for.

Once the check is performed correctly, a lit test needs to be updated,
since it is currently testing for the incorrect behaviour.

Diff Detail

Event Timeline

stepthomas created this revision.Jul 4 2023, 5:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 4 2023, 5:51 AM
stepthomas requested review of this revision.Jul 4 2023, 5:51 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 4 2023, 5:51 AM
foad accepted this revision.Jul 4 2023, 5:57 AM
foad added a reviewer: rampitec.

LGTM with or without nit.

llvm/test/CodeGen/AMDGPU/vcmpx-exec-war-hazard.mir
149

If you're renaming tests, maybe change this one to no_hazard_?

This revision is now accepted and ready to land.Jul 4 2023, 5:57 AM
stepthomas added inline comments.Jul 4 2023, 6:11 AM
llvm/test/CodeGen/AMDGPU/vcmpx-exec-war-hazard.mir
149

I did consider this, but decided that the test was illustrating a case where the was a hazard, it's just that the hazard recognizer had now correctly identified that it was already mitigated.

This revision was landed with ongoing or failed builds.Jul 4 2023, 6:44 AM
This revision was automatically updated to reflect the committed changes.