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

[Daniel Hoekwater via Phabricator via llvm-commits]

dhoekwater added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/TargetInstrInfo.h:607
   /// the offset of the position to insert the new branch.
-  virtual void insertIndirectBranch(MachineBasicBlock &MBB,
-                                    MachineBasicBlock &NewDestBB,
-                                    MachineBasicBlock &RestoreBB,
-                                    const DebugLoc &DL, int64_t BrOffset = 0,
-                                    RegScavenger *RS = nullptr) const {
+  virtual void
+  insertIndirectBranch(MachineBasicBlock &MBB, MachineBasicBlock &NewDestBB,
----------------
foad wrote:
> The new argument needs documentation.
Is it necessary to modify the interface of this function in this way? Since you're delegating the "is the new restore block identical to any old ones?" logic to the caller anyway, why not leave the caller responsible for deduplicating blocks?

If you were to add a new `virtual void retargetIndirectBranch(MachineBasicBlock &MBB, MachineBasicBlock &NewDestBB)` hook to `TargetInstrInfo`, you could keep the deduplication logic one level up by doing something like:
```
insertIndirectBranch(...);
if (!restoreBlock->empty() && isDuplicate(restoreBlock))
  retargetIndirectBranch(MBB, duplicateRestoreBlock);
```


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

https://reviews.llvm.org/D131587