[PATCH] D134950: [CodeGen] Use delegate to notify targets when virtual registers are created

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet accepted this revision.
qcolombet added a comment.
This revision is now accepted and ready to land.

LGTM, minor fixes inlined.



================
Comment at: llvm/include/llvm/CodeGen/MachineRegisterInfo.h:61
+    virtual void MRI_NotecloneVirtualRegister(Register ClonedReg,
+                                              Register Reg) {}
   };
----------------
Have the default implementation call `MRI_NoteNewVirtualRegister` and ditch the call to `MRI_NoteNewVirtualRegister` from `cloneVirtualRegister`.


================
Comment at: llvm/include/llvm/CodeGen/MachineRegisterInfo.h:61
+    virtual void MRI_NotecloneVirtualRegister(Register ClonedReg,
+                                              Register Reg) {}
   };
----------------
qcolombet wrote:
> Have the default implementation call `MRI_NoteNewVirtualRegister` and ditch the call to `MRI_NoteNewVirtualRegister` from `cloneVirtualRegister`.
Nit: I found `ClonedReg` to be ambiguous here: is it the result or the source of the cloning process?

Maybe use `NewReg` and `SrcReg`? (Ditto for `noteCloneVirtualRegister`)


================
Comment at: llvm/lib/CodeGen/MachineRegisterInfo.cpp:175
   setType(Reg, getType(VReg));
-  if (TheDelegate)
-    TheDelegate->MRI_NoteNewVirtualRegister(Reg);
+  noteNewVirtualRegister(Reg);
+  noteCloneVirtualRegister(Reg, VReg);
----------------
Ditch this one (to make sure we don't double notify delegates that listen to both events.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134950