[X86] Fix latent bugs in 32-bit CMPXCHG8B inserter

I found three issues:
1. the loop over E[ABCD]X copies run over BB start
2. the direct address of cmpxchg8b could be a frame index
3. the displacement of cmpxchg8b could be a global instead of an
   immediate

These were all introduced together in r287875, and should be fixed with
this change.

Issue reported by Zachary Turner.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@371678 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/lib/Target/X86/X86ISelLowering.cpp b/lib/Target/X86/X86ISelLowering.cpp
index 8cc4b5f56fe86879c382ad79e951b5b39f0f6831..5a274cb3e19b73eb4f88722f0a4ee6763beed5d8 100644 (file)
--- a/lib/Target/X86/X86ISelLowering.cpp
+++ b/lib/Target/X86/X86ISelLowering.cpp
@@ -31592,10 +31592,14 @@ X86TargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
     // After X86TargetLowering::ReplaceNodeResults CMPXCHG8B is glued to its
     // four operand definitions that are E[ABCD] registers. We skip them and
     // then insert the LEA.
-    MachineBasicBlock::iterator MBBI(MI);
-    while (MBBI->definesRegister(X86::EAX) || MBBI->definesRegister(X86::EBX) ||
-           MBBI->definesRegister(X86::ECX) || MBBI->definesRegister(X86::EDX))
-      --MBBI;
+    MachineBasicBlock::reverse_iterator RMBBI(MI.getReverseIterator());
+    while (RMBBI != BB->rend() && (RMBBI->definesRegister(X86::EAX) ||
+                                   RMBBI->definesRegister(X86::EBX) ||
+                                   RMBBI->definesRegister(X86::ECX) ||
+                                   RMBBI->definesRegister(X86::EDX))) {
+      ++RMBBI;
+    }
+    MachineBasicBlock::iterator MBBI(RMBBI);
     addFullAddress(
         BuildMI(*BB, *MBBI, DL, TII->get(X86::LEA32r), computedAddrVReg), AM);
 

diff --git a/lib/Target/X86/X86InstrBuilder.h b/lib/Target/X86/X86InstrBuilder.h
index 50aed98112c309ab3c29344144d2e4e74512e9d6..aa45e9b191c1c1100cb23f2a95c31227356393de 100644 (file)
--- a/lib/Target/X86/X86InstrBuilder.h
+++ b/lib/Target/X86/X86InstrBuilder.h
@@ -131,11 +131,11 @@ addDirectMem(const MachineInstrBuilder &MIB, unsigned Reg) {
 /// reference.
 static inline void setDirectAddressInInstr(MachineInstr *MI, unsigned Operand,
                                            unsigned Reg) {
-  // Direct memory address is in a form of: Reg, 1 (Scale), NoReg, 0, NoReg.
-  MI->getOperand(Operand).setReg(Reg);
+  // Direct memory address is in a form of: Reg/FI, 1 (Scale), NoReg, 0, NoReg.
+  MI->getOperand(Operand).ChangeToRegister(Reg, /*isDef=*/false);
   MI->getOperand(Operand + 1).setImm(1);
   MI->getOperand(Operand + 2).setReg(0);
-  MI->getOperand(Operand + 3).setImm(0);
+  MI->getOperand(Operand + 3).ChangeToImmediate(0);
   MI->getOperand(Operand + 4).setReg(0);
 }
 

diff --git a/test/CodeGen/X86/cmpxchg8b_alloca_regalloc_handling.ll b/test/CodeGen/X86/cmpxchg8b_alloca_regalloc_handling.ll
index b500484a4c895648d2ee4f199b177380c2a4893a..364d78aa31445426bfb1b537446d35e0c6c454cc 100644 (file)
--- a/test/CodeGen/X86/cmpxchg8b_alloca_regalloc_handling.ll
+++ b/test/CodeGen/X86/cmpxchg8b_alloca_regalloc_handling.ll
@@ -33,3 +33,64 @@ define void @foo_alloca_direct_address(i64* %addr, i32 %n) {
 ; CHECK-LABEL: foo_alloca_direct_address
 ; CHECK-NOT: leal    {{\(%e.*\)}}, [[REGISTER:%e.i]]
 ; CHECK: lock            cmpxchg8b       ([[REGISTER]])
+
+; We used to have a bug when combining:
+; - base pointer for stack frame (VLA + alignment)
+; - cmpxchg8b frameindex + index reg
+
+declare void @escape(i32*)
+
+define void @foo_alloca_index(i32 %i, i64 %val) {
+entry:
+  %Counters = alloca [19 x i64], align 32
+  %vla = alloca i32, i32 %i
+  call void @escape(i32* %vla)
+  br label %body
+
+body:
+  %p = getelementptr inbounds [19 x i64], [19 x i64]* %Counters, i32 0, i32 %i
+  %t2 = cmpxchg volatile i64* %p, i64 %val, i64 %val seq_cst seq_cst
+  %t3 = extractvalue { i64, i1 } %t2, 0
+  %cmp.i = icmp eq i64 %val, %t3
+  br i1 %cmp.i, label %done, label %body
+
+done:
+  ret void
+}
+
+; Check that we add a LEA
+; CHECK-LABEL: foo_alloca_index:
+; CHECK: leal    {{[0-9]*\(%e..,%e..,8\), %e..}}
+; CHECK: lock            cmpxchg8b       ({{%e..}})
+
+
+
+; We used to have a bug when combining:
+; - base pointer for stack frame (VLA + alignment)
+; - cmpxchg8b global + index reg
+
+@Counters = external global [19 x i64]
+
+define void @foo_alloca_index_global(i32 %i, i64 %val) {
+entry:
+  %aligner = alloca i32, align 32
+  call void @escape(i32* %aligner)
+  %vla = alloca i32, i32 %i
+  call void @escape(i32* %vla)
+  br label %body
+
+body:
+  %p = getelementptr inbounds [19 x i64], [19 x i64]* @Counters, i32 0, i32 %i
+  %t2 = cmpxchg volatile i64* %p, i64 %val, i64 %val seq_cst seq_cst
+  %t3 = extractvalue { i64, i1 } %t2, 0
+  %cmp.i = icmp eq i64 %val, %t3
+  br i1 %cmp.i, label %done, label %body
+
+done:
+  ret void
+}
+
+; Check that we add a LEA
+; CHECK-LABEL: foo_alloca_index_global:
+; CHECK: leal    {{Counters\(,%e..,8\), %e..}}
+; CHECK: lock            cmpxchg8b       ({{%e..}})