[PATCH] D131587: [CodeGen] Deduplicate restore blocks in branch relaxation

[Matt Arsenault via Phabricator via llvm-commits]

arsenm added a comment.

When would an inserted restore block ever not be identical with the same destination?



================
Comment at: llvm/lib/CodeGen/BranchRelaxation.cpp:94
+                                        ItEndNew = NewRestoreBB->end();
+      while (true) {
+        // Since target shouldn't place branches in the restore block, the only
----------------
This loop seems overcomplicated for a comparison. Can you just do a range loop, comparing identical instructions until the first terminator?


================
Comment at: llvm/lib/CodeGen/BranchRelaxation.cpp:98
+        // multiple different restore blocks. All but the last restore blocks
+        // has an unconditional branch to DestBB when they act as PrevBB.
+        // This branch doesn't take part in the check of whether two
----------------
s/has/have/


================
Comment at: llvm/lib/CodeGen/BranchRelaxation.cpp:105
+          return ItOld == ItEndOld && ItNew == ItEndNew;
+        if (!ItOld->isIdenticalTo(*ItNew))
+          return false;
----------------
Does this also need to consider the various flags on the block (e.g. alignment)?


================
Comment at: llvm/lib/Target/RISCV/RISCVInstrInfo.cpp:1010
+
+    MachineBasicBlock *JumpDest = DeduplicateRestoreBB(RestoreBB);
+    MI.getOperand(1).setMBB(JumpDest);
----------------
Why is this a callback? Why doesn't the pass just deduplicate to begin with and pass in the new block?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D131587/new/

https://reviews.llvm.org/D131587