This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Corrrectly emit AGPR copies in tryFoldPhiAGPR
ClosedPublic

Authored by Pierre-vh on Jul 12 2023, 6:10 AM.

Details

Reviewers
arsenm
Group Reviewers
Restricted Project
Commits
rG361e9eec515a: [AMDGPU] Corrrectly emit AGPR copies in tryFoldPhiAGPR
Summary
  • Don't create COPY instructions between PHI nodes.
  • Don't create V_ACCVGPR_WRITE with operands that aren't AGPR_32

Solves SWDEV-410408

Diff Detail

Event Timeline

Pierre-vh created this revision.Jul 12 2023, 6:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 12 2023, 6:10 AM
Pierre-vh requested review of this revision.Jul 12 2023, 6:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 12 2023, 6:10 AM
Pierre-vh updated this revision to Diff 539532.Jul 12 2023, 6:13 AM

Remove unnecessary format changes

arsenm added inline comments.Jul 12 2023, 7:28 AM
llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
1792

You want either getFirstNonPHI, SkipPHIsAndLabels or SkipPHIsLabelsAndDebug

1792

Probably SkipPHIsLabelsAndDebug

Pierre-vh updated this revision to Diff 539562.Jul 12 2023, 7:58 AM

Use correct function

arsenm accepted this revision.Jul 12 2023, 12:52 PM
arsenm added inline comments.
llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
1747

Class equality is usually wrong but probably happens to be OK here. ARC->IsSuperclassEq?

This revision is now accepted and ready to land.Jul 12 2023, 12:52 PM
Pierre-vh marked an inline comment as done.Jul 12 2023, 11:55 PM
Pierre-vh added inline comments.
llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
1747

I think an equality check is the right thing here. I only want to match AGPR32, nothing else, because V_ACCVGPR_WRITE only accepts that in its operands. A quick search for == &AMDGPU::AGPR_32RegClass across the codebase shows a few places that also use this pattern when dealing with that instruction

This revision was landed with ongoing or failed builds.Jul 12 2023, 11:55 PM
This revision was automatically updated to reflect the committed changes.
Pierre-vh marked an inline comment as done.