[PATCH] D67114: [mir-canon][NFC] Move MIR Vreg renaming code to separate file for better reuse.

[Puyan Lotfi via Phabricator via llvm-commits]

plotfi added inline comments.


================
Comment at: llvm/lib/CodeGen/MIRVRegNamerUtils.cpp:21
+/// in canoncal vreg renaming.
+std::vector<MachineInstr *> populateCandidates(MachineBasicBlock *MBB) {
+  std::vector<MachineInstr *> Candidates;
----------------
paquette wrote:
> Now that this isn't fully tied to the canonicalizer, I think it would make sense to give it a more descriptive name?
> 
> E.g. `findInstructionsWithSideEffects` or something?
I've decided to isolate these from the header file. So for now the names can remain without confusion. 


================
Comment at: llvm/lib/CodeGen/MIRVRegNamerUtils.cpp:51
+
+void doCandidateWalk(std::vector<TypedVReg> &VRegs,
+                     std::queue<TypedVReg> &RegQueue,
----------------
paquette wrote:
> Same thing here? Basically, anything with "candidate" makes less sense outside the context of a specific pass.
> 
> (Maybe it would make sense to pack all of this into a `Renamer` struct/class or something. I don't have any strong opinions there though.)
Agreed. I can consider changing everything named "candidate" to something else, but I'd rather do it in another commit. I've wrapped most of the functions into an anonymous namespace in the cpp file at this point. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67114