[PATCH] Remove redundent register mov by improving TwoAddressInstructionPass

[Quentin Colombet]

Hi Wei,

Inlined a couple of comments.
The main problem is that I believe my miss the general case here.

Thanks,
-Quentin


REPOSITORY
  rL LLVM

================
Comment at: lib/CodeGen/TwoAddressInstructionPass.cpp:318
@@ +317,3 @@
+MachineInstr* TwoAddressInstructionPass::getSingleDefUse(unsigned Reg, bool def) {
+  MachineInstr *ret = NULL;
+  for (MachineOperand &MO : MRI->reg_operands(Reg)) {
----------------
Use the API of MachineRegisterInfo to achieve this.
E.g, MRI has getUniqueVRegDef and use_nodgb_instructions.

================
Comment at: lib/CodeGen/TwoAddressInstructionPass.cpp:607
@@ +606,3 @@
+  MachineInstr *defC = getSingleDefUse(regC, true);
+  MachineInstr *useA = getSingleDefUse(regA, false);
+  if (useA && defC &&
----------------
Have you experiment with several users?
I.e., this code wouldn’t catch this:
  // %reg101 = MOV %reg100
  // %reg102 = ...
  // %reg103 = ADD %reg102, %reg101
  // %reg104 = <something> %reg103
  // %reg100 = MOV %reg103

Whereas this is fundamentally the same problem.

The bottom line is that this looks like a narrow case of a more general problem.
I’m fine moving incrementally, as long as you commit that you will working on this.

================
Comment at: lib/CodeGen/TwoAddressInstructionPass.cpp:616
@@ +615,3 @@
+      useA->getOperand(0).getReg() == defB->getOperand(1).getReg())
+    return false;
+
----------------
I would refactor this code a bit differently.

Something like.
if (useA && useA->isCopy) {
   unsigned DefCopyA = useA->getOperand(0).getReg();
  // Where match is the check for isCopy and getOperand(1).getReg() == DefCopyA
   if (match(DefCopyA, defB)
    return false;
   if (match(DefCopyA, defC)
    return true;
}

================
Comment at: test/CodeGen/X86/twoaddr-coalesce-3.ll:1
@@ +1,2 @@
+; RUN: llc < %s -march=x86 | grep mov | count 4
+
----------------
Use FileCheck and CHECK lines pleas.

================
Comment at: test/CodeGen/X86/twoaddr-coalesce-3.ll:7
@@ +6,3 @@
+; Function Attrs: nounwind uwtable
+define void @foo() #0 {
+entry:
----------------
Remove metadata and explain what this function is testing, so that future updates would be easier.

http://reviews.llvm.org/D7806

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/