[PATCH] D42402: [GISel]: Eliminate redundant copies b/w VRegs of same regclass at the end of InstructionSelection

[Justin Bogner via Phabricator via llvm-commits]

bogner added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/InstructionSelect.cpp:162
 
+  for (auto &MBI : MF) {
+    if (MBI.empty())
----------------
Why are you calling this MBI? It's a "MachineBasicBlock &", so it should really just be called MBB. I'd probably write out MachineBasicBlock instead of using auto here, as well.


================
Comment at: lib/CodeGen/GlobalISel/InstructionSelect.cpp:167
+    // Try to find redundant copies b/w vregs of the same register class.
+    MachineBasicBlock *MBB = &MBI;
+    bool ReachedBegin = false;
----------------
Don't bother getting a pointer here - just use the reference.


================
Comment at: lib/CodeGen/GlobalISel/InstructionSelect.cpp:179
+        --MII;
+      if (MI.getOpcode() == TargetOpcode::COPY) {
+        MachineRegisterInfo &MRI = MF.getRegInfo();
----------------
Might be more readable to use the early-continue style in all these conditions, ie:

  if (MI.getOpcode() != TargetOpcode::COPY)
    continue;

(Feel free to leave as is if that doesn't actually make it more readable)


================
Comment at: lib/CodeGen/GlobalISel/InstructionSelect.cpp:180
+      if (MI.getOpcode() == TargetOpcode::COPY) {
+        MachineRegisterInfo &MRI = MF.getRegInfo();
+        unsigned SrcReg = MI.getOperand(1).getReg();
----------------
This isn't used until after the next condition, best to move it down.


https://reviews.llvm.org/D42402