[llvm] r192636 - [X86][FastISel] During X86 fastisel, the address of indirect call was resolved

Quentin Colombet qcolombet at apple.com
Mon Oct 14 15:32:09 PDT 2013


Author: qcolombet
Date: Mon Oct 14 17:32:09 2013
New Revision: 192636

URL: http://llvm.org/viewvc/llvm-project?rev=192636&view=rev
Log:
[X86][FastISel] During X86 fastisel, the address of indirect call was resolved
through bitcast, ptrtoint, and inttoptr instructions. This is valid
only if the related instructions are in that same basic block, otherwise
we may reference variables that were not live accross basic blocks
resulting in undefined virtual registers.

The bug was exposed when both SDISel and FastISel were used within the same
function, i.e., one basic block is issued with FastISel and another with SDISel,
as demonstrated with the testcase.

<rdar://problem/15192473>

Added:
    llvm/trunk/test/CodeGen/X86/2013-10-14-FastISel-incorrect-vreg.ll
Modified:
    llvm/trunk/lib/Target/X86/X86FastISel.cpp

Modified: llvm/trunk/lib/Target/X86/X86FastISel.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FastISel.cpp?rev=192636&r1=192635&r2=192636&view=diff
==============================================================================
--- llvm/trunk/lib/Target/X86/X86FastISel.cpp (original)
+++ llvm/trunk/lib/Target/X86/X86FastISel.cpp Mon Oct 14 17:32:09 2013
@@ -632,9 +632,12 @@ redo_gep:
 bool X86FastISel::X86SelectCallAddress(const Value *V, X86AddressMode &AM) {
   const User *U = NULL;
   unsigned Opcode = Instruction::UserOp1;
-  if (const Instruction *I = dyn_cast<Instruction>(V)) {
+  const Instruction *I = dyn_cast<Instruction>(V);
+  bool InMBB = true;
+  if (I) {
     Opcode = I->getOpcode();
     U = I;
+    InMBB = I->getParent() == FuncInfo.MBB->getBasicBlock();
   } else if (const ConstantExpr *C = dyn_cast<ConstantExpr>(V)) {
     Opcode = C->getOpcode();
     U = C;
@@ -643,18 +646,22 @@ bool X86FastISel::X86SelectCallAddress(c
   switch (Opcode) {
   default: break;
   case Instruction::BitCast:
-    // Look past bitcasts.
-    return X86SelectCallAddress(U->getOperand(0), AM);
+    // Look past bitcasts if its operand is in the same BB.
+    if (InMBB)
+      return X86SelectCallAddress(U->getOperand(0), AM);
+    break;
 
   case Instruction::IntToPtr:
-    // Look past no-op inttoptrs.
-    if (TLI.getValueType(U->getOperand(0)->getType()) == TLI.getPointerTy())
+    // Look past no-op inttoptrs if its operand is in the same BB.
+    if (InMBB &&
+        TLI.getValueType(U->getOperand(0)->getType()) == TLI.getPointerTy())
       return X86SelectCallAddress(U->getOperand(0), AM);
     break;
 
   case Instruction::PtrToInt:
-    // Look past no-op ptrtoints.
-    if (TLI.getValueType(U->getType()) == TLI.getPointerTy())
+    // Look past no-op ptrtoints if its operand is in the same BB.
+    if (InMBB &&
+        TLI.getValueType(U->getType()) == TLI.getPointerTy())
       return X86SelectCallAddress(U->getOperand(0), AM);
     break;
   }

Added: llvm/trunk/test/CodeGen/X86/2013-10-14-FastISel-incorrect-vreg.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/2013-10-14-FastISel-incorrect-vreg.ll?rev=192636&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/X86/2013-10-14-FastISel-incorrect-vreg.ll (added)
+++ llvm/trunk/test/CodeGen/X86/2013-10-14-FastISel-incorrect-vreg.ll Mon Oct 14 17:32:09 2013
@@ -0,0 +1,132 @@
+; RUN: llc -mtriple x86_64-apple-darwin -O0 < %s -o - | FileCheck %s
+;
+; During X86 fastisel, the address of indirect call was resolved
+; through bitcast, ptrtoint, and inttoptr instructions. This is valid
+; only if the related instructions are in that same basic block, otherwise
+; we may reference variables that were not live accross basic blocks
+; resulting in undefined virtual registers.
+;
+; In this example, this is illustrated by a the spill/reload of the
+; LOADED_PTR_SLOT.
+;
+; Before this patch, the compiler was accessing two different spill
+; slots.
+; <rdar://problem/15192473>
+
+; CHECK-LABEL: @test_bitcast
+; Load the value of the function pointer: %loaded_ptr
+; CHECK: movq (%rdi), [[LOADED_PTR:%[a-z]+]]
+; Spill %arg2.
+; CHECK: movq %rdx, [[ARG2_SLOT:[0-9]*\(%[a-z]+\)]]
+; Spill %loaded_ptr.
+; CHECK: movq [[LOADED_PTR]], [[LOADED_PTR_SLOT:[0-9]*\(%[a-z]+\)]]
+; Perform the indirect call.
+; Load the first argument
+; CHECK: movq [[ARG2_SLOT]], %rdi
+; Load the second argument
+; CHECK: movq [[ARG2_SLOT]], %rsi
+; Load the thrid argument
+; CHECK: movq [[ARG2_SLOT]], %rdx
+; Load the function pointer.
+; CHECK: movq [[LOADED_PTR_SLOT]], [[FCT_PTR:%[a-z]+]]
+; Call.
+; CHECK: callq *[[FCT_PTR]]
+; CHECK: ret
+define i64 @test_bitcast(i64 (i64, i64, i64)** %arg, i1 %bool, i64 %arg2) {
+entry:
+  %loaded_ptr = load i64 (i64, i64, i64)** %arg, align 8
+  %raw = bitcast i64 (i64, i64, i64)* %loaded_ptr to i8*
+  switch i1 %bool, label %default [
+    i1 true, label %label_true
+    i1 false, label %label_end
+  ]
+default:
+  unreachable
+
+label_true:
+  br label %label_end
+
+label_end:
+  %fct_ptr = bitcast i8* %raw to i64 (i64, i64, i64)*
+  %res = call i64 %fct_ptr(i64 %arg2, i64 %arg2, i64 %arg2)
+  ret i64 %res
+}
+
+; CHECK-LABEL: @test_inttoptr
+; Load the value of the function pointer: %loaded_ptr
+; CHECK: movq (%rdi), [[LOADED_PTR:%[a-z]+]]
+; Spill %arg2.
+; CHECK: movq %rdx, [[ARG2_SLOT:[0-9]*\(%[a-z]+\)]]
+; Spill %loaded_ptr.
+; CHECK: movq [[LOADED_PTR]], [[LOADED_PTR_SLOT:[0-9]*\(%[a-z]+\)]]
+; Perform the indirect call.
+; Load the first argument
+; CHECK: movq [[ARG2_SLOT]], %rdi
+; Load the second argument
+; CHECK: movq [[ARG2_SLOT]], %rsi
+; Load the thrid argument
+; CHECK: movq [[ARG2_SLOT]], %rdx
+; Load the function pointer.
+; CHECK: movq [[LOADED_PTR_SLOT]], [[FCT_PTR:%[a-z]+]]
+; Call.
+; CHECK: callq *[[FCT_PTR]]
+; CHECK: ret
+define i64 @test_inttoptr(i64 (i64, i64, i64)** %arg, i1 %bool, i64 %arg2) {
+entry:
+  %loaded_ptr = load i64 (i64, i64, i64)** %arg, align 8
+  %raw = ptrtoint i64 (i64, i64, i64)* %loaded_ptr to i64
+  switch i1 %bool, label %default [
+    i1 true, label %label_true
+    i1 false, label %label_end
+  ]
+default:
+  unreachable
+
+label_true:
+  br label %label_end
+
+label_end:
+  %fct_ptr = inttoptr i64 %raw to i64 (i64, i64, i64)*
+  %res = call i64 %fct_ptr(i64 %arg2, i64 %arg2, i64 %arg2)
+  ret i64 %res
+}
+
+; CHECK-LABEL: @test_ptrtoint
+; Load the value of the function pointer: %loaded_ptr
+; CHECK: movq (%rdi), [[LOADED_PTR:%[a-z]+]]
+; Spill %arg2.
+; CHECK: movq %rdx, [[ARG2_SLOT:[0-9]*\(%[a-z]+\)]]
+; Spill %loaded_ptr.
+; CHECK: movq [[LOADED_PTR]], [[LOADED_PTR_SLOT:[0-9]*\(%[a-z]+\)]]
+; Perform the indirect call.
+; Load the first argument
+; CHECK: movq [[ARG2_SLOT]], %rdi
+; Load the second argument
+; CHECK: movq [[ARG2_SLOT]], %rsi
+; Load the thrid argument
+; CHECK: movq [[ARG2_SLOT]], %rdx
+; Load the function pointer.
+; CHECK: movq [[LOADED_PTR_SLOT]], [[FCT_PTR:%[a-z]+]]
+; Call.
+; CHECK: callq *[[FCT_PTR]]
+; CHECK: ret
+define i64 @test_ptrtoint(i64 (i64, i64, i64)** %arg, i1 %bool, i64 %arg2) {
+entry:
+  %loaded_ptr = load i64 (i64, i64, i64)** %arg, align 8
+  %raw = bitcast i64 (i64, i64, i64)* %loaded_ptr to i8*
+  switch i1 %bool, label %default [
+    i1 true, label %label_true
+    i1 false, label %label_end
+  ]
+default:
+  unreachable
+
+label_true:
+  br label %label_end
+
+label_end:
+  %fct_int = ptrtoint i8* %raw to i64
+  %fct_ptr = inttoptr i64 %fct_int to i64 (i64, i64, i64)*
+  %res = call i64 %fct_ptr(i64 %arg2, i64 %arg2, i64 %arg2)
+  ret i64 %res
+}





More information about the llvm-commits mailing list