MachineFunctionPass: Clear properties before running function

[arsenm]

This ensures !isSSA checks in the function work if the input MIR happened to appear as SSA.

[llvmbot]

@llvm/pr-subscribers-llvm-regalloc

Changes

This ensures !isSSA checks in the function work if the input MIR happened to appear as SSA.

---

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

2 Files Affected:

• (modified) llvm/lib/CodeGen/MachineFunctionPass.cpp (+2-1)
• (modified) llvm/lib/CodeGen/RegisterCoalescer.cpp (-8)

diff --git a/llvm/lib/CodeGen/MachineFunctionPass.cpp b/llvm/lib/CodeGen/MachineFunctionPass.cpp
index 3a1e1720be9c627..d57a912f418b728 100644
--- a/llvm/lib/CodeGen/MachineFunctionPass.cpp
+++ b/llvm/lib/CodeGen/MachineFunctionPass.cpp
@@ -88,6 +88,8 @@ bool MachineFunctionPass::runOnFunction(Function &F) {
     MF.print(OS);
   }
 
+  MFProps.reset(ClearedProperties);
+
   bool RV = runOnMachineFunction(MF);
 
   if (ShouldEmitSizeRemarks) {
@@ -114,7 +116,6 @@ bool MachineFunctionPass::runOnFunction(Function &F) {
   }
 
   MFProps.set(SetProperties);
-  MFProps.reset(ClearedProperties);
 
   // For --print-changed, print if the serialized MF has changed. Modes other
   // than quiet/verbose are unimplemented and treated the same as 'quiet'.
diff --git a/llvm/lib/CodeGen/RegisterCoalescer.cpp b/llvm/lib/CodeGen/RegisterCoalescer.cpp
index 516095a699ea1e8..238462113bfe78c 100644
--- a/llvm/lib/CodeGen/RegisterCoalescer.cpp
+++ b/llvm/lib/CodeGen/RegisterCoalescer.cpp
@@ -4131,14 +4131,6 @@ bool RegisterCoalescer::runOnMachineFunction(MachineFunction &fn) {
   else
     JoinGlobalCopies = (EnableGlobalCopies == cl::BOU_TRUE);
 
-  // FIXME: MachineFunctionProperties cannot express the required pre-property
-  // no-SSA. When running a MIR testcase without any virtual register defs, the
-  // MIR parser assumes SSA. MachineFunctionPass::getClearedProperties is called
-  // after the pass is run, so the properties at this point say it's an SSA
-  // function.  Forcibly clear it here so -verify-coalescing doesn't complain
-  // after multiple virtual register defs are introduced.
-  MRI->leaveSSA();
-
   // If there are PHIs tracked by debug-info, they will need updating during
   // coalescing. Build an index of those PHIs to ease updating.
   SlotIndexes *Slots = LIS->getSlotIndexes();

[MatzeB]

I think complained when we introduced MachineFunctionPass::getClearedProperties() / setProperties() that we should not have that function and rather clear things within MachineFunction::runOnMachineFunction at the apropriate time (but I think I was overruled in review).

I feel slightly uneasy that moving the clearing to before the function will just result in different problems for other passes.

Though pragmatically if we cannot get consensus to remove getClearedProperties() and getSetProperties APIs in their current form and you can make the tests pass then may as well go for it...

[MatzeB]

I think the whole getClearedProperties() and getSetProperties() design is an error and we should rather set/clear properties as necessary within runOnMachineFunction.

Short of changing this I guess I am fine with this when tests pass. LGTM