[llvm] [AArch64] Fix tryMergeAdjacentSTG function in PrologEpilog pass (PR #68873)

via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 23 11:18:40 PDT 2023


================
@@ -3763,6 +3774,33 @@ MachineBasicBlock::iterator tryMergeAdjacentSTG(MachineBasicBlock::iterator II,
   MachineBasicBlock::iterator InsertI = Instrs.back().MI;
   InsertI++;
 
+  // All the gathered stack tag instructions are merged and placed after
+  // last tag store in the list. The check should be made if the nzcv
+  // flag that we might be overriding will affect upcoming instructions
+  // that reads the flag(Here NZCV) or if the register is live out. If
+  // the flag is defined again we don't have this problem as the flag
+  // is anyway subject to modification.
+  // Though there could be different ways to curb this maltransformation,
+  // this seems to be simple and straight forward without changing
+  // existing algorithm.
+  // FIXME : This approach of bailing out from merge is also conservative
+  // in some ways like even if stg loops are not present after merge
+  // these checks are done (which are not needed).
+  bool modifiesFirst = false, readsFirst = false;
+  for (MachineBasicBlock::iterator I = InsertI, E = MBB->end(); I != E; I++) {
+    MachineInstr &MI = *I;
+    if (MI.readsRegister(AArch64::NZCV)) {
+      readsFirst = true;
+      break;
+    }
+    if (MI.modifiesRegister(AArch64::NZCV)) {
+      modifiesFirst = true;
+      break;
+    }
+  }
+  if (!modifiesFirst && (readsFirst || isNZCVLiveOut(*MBB)))
----------------
kartcq wrote:

Thanks for taking the time to look into this @kbeyls. Yes you are right. From what I understand first two conditions are most accurate to block the merging. But the third condition is kind of superset of both. Once we are steer clear of the third the first two should not matter. That's the reason I mentioned the patch is conservative and might block merging at places where we need not to. But Checking Liveness ensures safety and avoid mis-compilation even if we did not include all possible conditions right now. 
Also I am trying to replace the Liveness logic I put above using the LivePhysRegs class APIs. I am seeing some crashed while running tests. Debugging on that.

https://github.com/llvm/llvm-project/pull/68873


More information about the llvm-commits mailing list