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

[Aditya Nandakumar via Phabricator via llvm-commits]

aditya_nandakumar added inline comments.


================
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;
----------------
bogner wrote:
> Don't bother getting a pointer here - just use the reference.
Will update.


================
Comment at: lib/CodeGen/GlobalISel/InstructionSelect.cpp:179
+        --MII;
+      if (MI.getOpcode() == TargetOpcode::COPY) {
+        MachineRegisterInfo &MRI = MF.getRegInfo();
----------------
bogner wrote:
> 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)
Sure. I can update to that.


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


================
Comment at: lib/CodeGen/GlobalISel/InstructionSelect.cpp:194
+    }
+  }
+
----------------
qcolombet wrote:
> Instead of having another pass through the IR for this, could we do it while we do the ISel loop?
> 
> Meaning, if the register banks match, push the RC to the def and let constraint operand create a new copy if need be.
> I would expect this to be more efficient in term of compile time, but we would need to check.
That sounds like it might work - let me give that a try.


https://reviews.llvm.org/D42402