AMDGPU: Use MFPropsModifier modifier in SIFoldOperands

[arsenm]

This doesn't appear to work. I do not get an error in the new PM.

[arsenm]

• AMDGPU: Use MFPropsModifier modifier in SIFoldOperands #127752 👈 (View in Graphite)
• main

This stack of pull requests is managed by Graphite. Learn more about stacking.

[llvmbot]

@llvm/pr-subscribers-backend-amdgpu

Author: Matt Arsenault (arsenm)

Changes

This doesn't appear to work. I do not get an error in the new PM.

---

Full diff: https://github.com/llvm/llvm-project/pull/127752.diff

2 Files Affected:

• (modified) llvm/lib/Target/AMDGPU/SIFoldOperands.cpp (+2)
• (added) llvm/test/CodeGen/AMDGPU/si-fold-operands-requires-ssa.mir (+15)

diff --git a/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp b/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
index ab396929162d0..54baf90d95a12 100644
--- a/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
@@ -2311,6 +2311,8 @@ bool SIFoldOperandsImpl::tryOptimizeAGPRPhis(MachineBasicBlock &MBB) {
 }
 
 bool SIFoldOperandsImpl::run(MachineFunction &MF) {
+  MFPropsModifier _(*this, MF);
+
   MRI = &MF.getRegInfo();
   ST = &MF.getSubtarget<GCNSubtarget>();
   TII = ST->getInstrInfo();
diff --git a/llvm/test/CodeGen/AMDGPU/si-fold-operands-requires-ssa.mir b/llvm/test/CodeGen/AMDGPU/si-fold-operands-requires-ssa.mir
new file mode 100644
index 0000000000000..9cad4eeab76c8
--- /dev/null
+++ b/llvm/test/CodeGen/AMDGPU/si-fold-operands-requires-ssa.mir
@@ -0,0 +1,15 @@
+# RUN: not --crash llc -mtriple=amdgcn -mcpu=gfx900 -run-pass=si-fold-operands -filetype=null %s 2>&1 | FileCheck -check-prefix=ERR %s
+# RUN: not --crash llc -mtriple=amdgcn -mcpu=gfx900 -passes=si-fold-operands -filetype=null %s 2>&1 | FileCheck -check-prefix=ERR %s
+
+# ERR: MachineFunctionProperties required by SI Fold Operands pass are not met by function not_ssa.
+# ERR-NEXT: Required properties: IsSSA
+# ERR-NEXT: Current properties: NoPHIs
+---
+name:            not_ssa
+body:             |
+  bb.0:
+    liveins: $vgpr0, $vgpr1
+    %0:vgpr_32 = COPY $vgpr0
+    %0:vgpr_32 = COPY $vgpr1
+
+...

[optimisan]

Taking a look.

[optimisan]

Turned out the error printing is finicky.

[arsenm]

Turned out the error printing is finicky.

There is still no error with the new PM

[arsenm]

ping

[optimisan]

There is still no error with the new PM

This is the error right? The test passes now.

[arsenm]

No, the test is failing on the bot. And fails for me locally. There is no error produced

[optimisan]

Right, checked again. Will take a look

[optimisan]

Moved MFPropsModifier to NPM's run method.

[arsenm]

Moved MFPropsModifier to NPM's run method.

I think this is a decent example of why this is a terrible API. There's no enforcement that every pass uses it

[dyung]

Hi @arsenm, the test added in this change seems to be failing on a build bot (https://lab.llvm.org/buildbot/#/builders/202/builds/100), can you take a look?

[arsenm]

Apparently this error is only produced with asserts, but we should promote this to an unconditional report_fatal_error

[arsenm]

Fixed in 092e255