[llvm] 5b8befd - [X86] Special-case ADD of two identical registers in convertToThreeAddress

Jay Foad via llvm-commits llvm-commits at lists.llvm.org
Thu Oct 7 11:50:34 PDT 2021


Author: Jay Foad
Date: 2021-10-07T19:50:27+01:00
New Revision: 5b8befdd026d75562f127a34e0b0584820b03581

URL: https://github.com/llvm/llvm-project/commit/5b8befdd026d75562f127a34e0b0584820b03581
DIFF: https://github.com/llvm/llvm-project/commit/5b8befdd026d75562f127a34e0b0584820b03581.diff

LOG: [X86] Special-case ADD of two identical registers in convertToThreeAddress

X86InstrInfo::convertToThreeAddress would convert this:

  %1:gr32 = ADD32rr killed %0:gr32(tied-def 0), %0:gr32, implicit-def dead $eflags

to this:

  undef %2.sub_32bit:gr64 = COPY killed %0:gr32
  undef %3.sub_32bit:gr64_nosp = COPY %0:gr32
  %1:gr32 = LEA64_32r killed %2:gr64, 1, killed %3:gr64_nosp, 0, $noreg

Note that in the ADD32rr, %0 was used twice and the first use had a kill
flag, which is what MachineInstr::addRegisterKilled does.

In the converted code, each use of %0 is copied to a new reg, and the
first COPY inherits the kill flag from the ADD32rr. This causes
machine verification to fail (if you force it to run after
TwoAddressInstructionPass) because the second COPY uses %0 after it is
killed. Note that machine verification is currently disabled after
TwoAddressInstructionPass but this is a step towards being able to
enable it.

Fix this by not inserting more than one COPY from the same source
register.

Differential Revision: https://reviews.llvm.org/D110829

Added: 
    

Modified: 
    llvm/lib/Target/X86/X86InstrInfo.cpp
    llvm/test/CodeGen/X86/twoaddr-mul2.mir

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/X86/X86InstrInfo.cpp b/llvm/lib/Target/X86/X86InstrInfo.cpp
index 71136130ce7b8..e06202d378a6a 100644
--- a/llvm/lib/Target/X86/X86InstrInfo.cpp
+++ b/llvm/lib/Target/X86/X86InstrInfo.cpp
@@ -1205,12 +1205,12 @@ bool X86InstrInfo::classifyLEAReg(MachineInstr &MI, const MachineOperand &Src,
       &X86::GR64_NOSPRegClass : &X86::GR32_NOSPRegClass;
   }
   Register SrcReg = Src.getReg();
+  isKill = MI.killsRegister(SrcReg);
 
   // For both LEA64 and LEA32 the register already has essentially the right
   // type (32-bit or 64-bit) we may just need to forbid SP.
   if (Opc != X86::LEA64_32r) {
     NewSrc = SrcReg;
-    isKill = Src.isKill();
     assert(!Src.isUndef() && "Undef op doesn't need optimization");
 
     if (NewSrc.isVirtual() && !MF.getRegInfo().constrainRegClass(NewSrc, RC))
@@ -1225,8 +1225,7 @@ bool X86InstrInfo::classifyLEAReg(MachineInstr &MI, const MachineOperand &Src,
     ImplicitOp = Src;
     ImplicitOp.setImplicit();
 
-    NewSrc = getX86SubSuperRegister(Src.getReg(), 64);
-    isKill = Src.isKill();
+    NewSrc = getX86SubSuperRegister(SrcReg, 64);
     assert(!Src.isUndef() && "Undef op doesn't need optimization");
   } else {
     // Virtual register of the wrong class, we have to create a temporary 64-bit
@@ -1235,7 +1234,7 @@ bool X86InstrInfo::classifyLEAReg(MachineInstr &MI, const MachineOperand &Src,
     MachineInstr *Copy =
         BuildMI(*MI.getParent(), MI, MI.getDebugLoc(), get(TargetOpcode::COPY))
             .addReg(NewSrc, RegState::Define | RegState::Undef, X86::sub_32bit)
-            .add(Src);
+            .addReg(SrcReg, getKillRegState(isKill));
 
     // Which is obviously going to be dead after we're done with it.
     isKill = true;
@@ -1532,13 +1531,6 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
     else
       Opc = Is64Bit ? X86::LEA64_32r : X86::LEA32r;
 
-    bool isKill;
-    Register SrcReg;
-    MachineOperand ImplicitOp = MachineOperand::CreateReg(0, false);
-    if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/ true,
-                        SrcReg, isKill, ImplicitOp, LV))
-      return nullptr;
-
     const MachineOperand &Src2 = MI.getOperand(2);
     bool isKill2;
     Register SrcReg2;
@@ -1547,6 +1539,20 @@ MachineInstr *X86InstrInfo::convertToThreeAddress(MachineInstr &MI,
                         SrcReg2, isKill2, ImplicitOp2, LV))
       return nullptr;
 
+    bool isKill;
+    Register SrcReg;
+    MachineOperand ImplicitOp = MachineOperand::CreateReg(0, false);
+    if (Src.getReg() == Src2.getReg()) {
+      // Don't call classify LEAReg a second time on the same register, in case
+      // the first call inserted a COPY from Src2 and marked it as killed.
+      isKill = isKill2;
+      SrcReg = SrcReg2;
+    } else {
+      if (!classifyLEAReg(MI, Src, Opc, /*AllowSP=*/true,
+                          SrcReg, isKill, ImplicitOp, LV))
+        return nullptr;
+    }
+
     MachineInstrBuilder MIB = BuildMI(MF, MI.getDebugLoc(), get(Opc)).add(Dest);
     if (ImplicitOp.getReg() != 0)
       MIB.add(ImplicitOp);

diff  --git a/llvm/test/CodeGen/X86/twoaddr-mul2.mir b/llvm/test/CodeGen/X86/twoaddr-mul2.mir
index f8f2e063d3e4b..5aa9613e162eb 100644
--- a/llvm/test/CodeGen/X86/twoaddr-mul2.mir
+++ b/llvm/test/CodeGen/X86/twoaddr-mul2.mir
@@ -1,7 +1,7 @@
 # NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
-# RUN: llc -mtriple=x86_64-unknown -mcpu=haswell -run-pass=twoaddressinstruction %s -o - | FileCheck %s
+# RUN: llc -mtriple=x86_64-unknown -mcpu=haswell -run-pass=twoaddressinstruction -verify-machineinstrs %s -o - | FileCheck %s
 
-# FIXME: The killed flag should be on the second COPY from [[COPY]], not the first one.
+# Check that we don't have any uses of [[COPY]] after it is killed.
 ---
 name: test_mul_by_2
 tracksRegLiveness: true
@@ -13,9 +13,8 @@ body: |
     ; CHECK: liveins: $edi
     ; CHECK-NEXT: {{  $}}
     ; CHECK-NEXT: [[COPY:%[0-9]+]]:gr32 = COPY killed $edi
-    ; CHECK-NEXT: undef %2.sub_32bit:gr64 = COPY killed [[COPY]]
-    ; CHECK-NEXT: undef %3.sub_32bit:gr64_nosp = COPY [[COPY]]
-    ; CHECK-NEXT: [[LEA64_32r:%[0-9]+]]:gr32 = LEA64_32r killed %2, 1, killed %3, 0, $noreg
+    ; CHECK-NEXT: undef %2.sub_32bit:gr64_nosp = COPY killed [[COPY]]
+    ; CHECK-NEXT: [[LEA64_32r:%[0-9]+]]:gr32 = LEA64_32r killed %2, 1, killed %2, 0, $noreg
     ; CHECK-NEXT: $eax = COPY killed [[LEA64_32r]]
     ; CHECK-NEXT: RET 0, killed $eax
     %0:gr32 = COPY killed $edi


        


More information about the llvm-commits mailing list