<html><head><meta http-equiv="Content-Type" content="text/html charset=iso-8859-1"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;"><div><div apple-content-edited="true"><div style="color: rgb(0, 0, 0); font-family: Helvetica; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: 2; text-align: -webkit-auto; text-indent: 0px; text-transform: none; white-space: normal; widows: 2; word-spacing: 0px; -webkit-text-size-adjust: auto; -webkit-text-stroke-width: 0px; word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "></div>
</div>
<br><div><div>On Oct 22, 2013, at 1:34 PM, Eric Christopher <<a href="mailto:echristo@gmail.com">echristo@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite">I seem to want the check-in comment somewhere in the code highlighting<br>the problem that would occur otherwise, but I guess having a failing<br>test is good enough. Anyhow, just a "if you can come up with some way<br>to elaborate in the code then awesome" comment :)<br><br>-eric<br><br>On Mon, Oct 14, 2013 at 3:32 PM, Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>> wrote:<br><blockquote type="cite">Author: qcolombet<br>Date: Mon Oct 14 17:32:09 2013<br>New Revision: 192636<br><br>URL: <a href="http://llvm.org/viewvc/llvm-project?rev=192636&view=rev">http://llvm.org/viewvc/llvm-project?rev=192636&view=rev</a><br>Log:<br>[X86][FastISel] During X86 fastisel, the address of indirect call was resolved<br>through bitcast, ptrtoint, and inttoptr instructions. This is valid<br>only if the related instructions are in that same basic block, otherwise<br>we may reference variables that were not live accross basic blocks<br>resulting in undefined virtual registers.<br><br>The bug was exposed when both SDISel and FastISel were used within the same<br>function, i.e., one basic block is issued with FastISel and another with SDISel,<br>as demonstrated with the testcase.<br><br><<a href="rdar://problem/15192473">rdar://problem/15192473</a>><br><br>Added:<br> llvm/trunk/test/CodeGen/X86/2013-10-14-FastISel-incorrect-vreg.ll<br>Modified:<br> llvm/trunk/lib/Target/X86/X86FastISel.cpp<br><br>Modified: llvm/trunk/lib/Target/X86/X86FastISel.cpp<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FastISel.cpp?rev=192636&r1=192635&r2=192636&view=diff">http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FastISel.cpp?rev=192636&r1=192635&r2=192636&view=diff</a><br>==============================================================================<br>--- llvm/trunk/lib/Target/X86/X86FastISel.cpp (original)<br>+++ llvm/trunk/lib/Target/X86/X86FastISel.cpp Mon Oct 14 17:32:09 2013<br>@@ -632,9 +632,12 @@ redo_gep:<br> bool X86FastISel::X86SelectCallAddress(const Value *V, X86AddressMode &AM) {<br> const User *U = NULL;<br> unsigned Opcode = Instruction::UserOp1;<br>- if (const Instruction *I = dyn_cast<Instruction>(V)) {<br>+ const Instruction *I = dyn_cast<Instruction>(V);<br>+ bool InMBB = true;<br>+ if (I) {<br> Opcode = I->getOpcode();<br> U = I;<br>+ InMBB = I->getParent() == FuncInfo.MBB->getBasicBlock();<br> } else if (const ConstantExpr *C = dyn_cast<ConstantExpr>(V)) {<br> Opcode = C->getOpcode();<br> U = C;<br>@@ -643,18 +646,22 @@ bool X86FastISel::X86SelectCallAddress(c<br> switch (Opcode) {<br> default: break;<br> case Instruction::BitCast:<br>- // Look past bitcasts.<br>- return X86SelectCallAddress(U->getOperand(0), AM);<br>+ // Look past bitcasts if its operand is in the same BB.<br>+ if (InMBB)<br>+ return X86SelectCallAddress(U->getOperand(0), AM);<br>+ break;<br><br> case Instruction::IntToPtr:<br>- // Look past no-op inttoptrs.<br>- if (TLI.getValueType(U->getOperand(0)->getType()) == TLI.getPointerTy())<br>+ // Look past no-op inttoptrs if its operand is in the same BB.<br>+ if (InMBB &&<br>+ TLI.getValueType(U->getOperand(0)->getType()) == TLI.getPointerTy())<br> return X86SelectCallAddress(U->getOperand(0), AM);<br> break;<br><br> case Instruction::PtrToInt:<br>- // Look past no-op ptrtoints.<br>- if (TLI.getValueType(U->getType()) == TLI.getPointerTy())<br>+ // Look past no-op ptrtoints if its operand is in the same BB.<br>+ if (InMBB &&<br>+ TLI.getValueType(U->getType()) == TLI.getPointerTy())<br> return X86SelectCallAddress(U->getOperand(0), AM);<br> break;<br> }<br><br>Added: llvm/trunk/test/CodeGen/X86/2013-10-14-FastISel-incorrect-vreg.ll<br>URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/2013-10-14-FastISel-incorrect-vreg.ll?rev=192636&view=auto">http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/2013-10-14-FastISel-incorrect-vreg.ll?rev=192636&view=auto</a><br>==============================================================================<br>--- llvm/trunk/test/CodeGen/X86/2013-10-14-FastISel-incorrect-vreg.ll (added)<br>+++ llvm/trunk/test/CodeGen/X86/2013-10-14-FastISel-incorrect-vreg.ll Mon Oct 14 17:32:09 2013<br>@@ -0,0 +1,132 @@<br>+; RUN: llc -mtriple x86_64-apple-darwin -O0 < %s -o - | FileCheck %s<br>+;<br>+; During X86 fastisel, the address of indirect call was resolved<br>+; through bitcast, ptrtoint, and inttoptr instructions. This is valid<br>+; only if the related instructions are in that same basic block, otherwise<br>+; we may reference variables that were not live accross basic blocks<br>+; resulting in undefined virtual registers.<br>+;<br>+; In this example, this is illustrated by a the spill/reload of the<br>+; LOADED_PTR_SLOT.<br>+;<br>+; Before this patch, the compiler was accessing two different spill<br>+; slots.<br>+; <<a href="rdar://problem/15192473">rdar://problem/15192473</a>><br>+<br>+; CHECK-LABEL: @test_bitcast<br>+; Load the value of the function pointer: %loaded_ptr<br>+; CHECK: movq (%rdi), [[LOADED_PTR:%[a-z]+]]<br>+; Spill %arg2.<br>+; CHECK: movq %rdx, [[ARG2_SLOT:[0-9]*\(%[a-z]+\)]]<br>+; Spill %loaded_ptr.<br>+; CHECK: movq [[LOADED_PTR]], [[LOADED_PTR_SLOT:[0-9]*\(%[a-z]+\)]]<br>+; Perform the indirect call.<br>+; Load the first argument<br>+; CHECK: movq [[ARG2_SLOT]], %rdi<br>+; Load the second argument<br>+; CHECK: movq [[ARG2_SLOT]], %rsi<br>+; Load the thrid argument<br>+; CHECK: movq [[ARG2_SLOT]], %rdx<br>+; Load the function pointer.<br>+; CHECK: movq [[LOADED_PTR_SLOT]], [[FCT_PTR:%[a-z]+]]<br>+; Call.<br>+; CHECK: callq *[[FCT_PTR]]<br>+; CHECK: ret<br>+define i64 @test_bitcast(i64 (i64, i64, i64)** %arg, i1 %bool, i64 %arg2) {<br>+entry:<br>+ %loaded_ptr = load i64 (i64, i64, i64)** %arg, align 8<br>+ %raw = bitcast i64 (i64, i64, i64)* %loaded_ptr to i8*<br>+ switch i1 %bool, label %default [<br>+ i1 true, label %label_true<br>+ i1 false, label %label_end<br>+ ]<br>+default:<br>+ unreachable<br>+<br>+label_true:<br>+ br label %label_end<br>+<br>+label_end:<br>+ %fct_ptr = bitcast i8* %raw to i64 (i64, i64, i64)*<br>+ %res = call i64 %fct_ptr(i64 %arg2, i64 %arg2, i64 %arg2)<br>+ ret i64 %res<br>+}<br>+<br>+; CHECK-LABEL: @test_inttoptr<br>+; Load the value of the function pointer: %loaded_ptr<br>+; CHECK: movq (%rdi), [[LOADED_PTR:%[a-z]+]]<br>+; Spill %arg2.<br>+; CHECK: movq %rdx, [[ARG2_SLOT:[0-9]*\(%[a-z]+\)]]<br>+; Spill %loaded_ptr.<br>+; CHECK: movq [[LOADED_PTR]], [[LOADED_PTR_SLOT:[0-9]*\(%[a-z]+\)]]<br>+; Perform the indirect call.<br>+; Load the first argument<br>+; CHECK: movq [[ARG2_SLOT]], %rdi<br>+; Load the second argument<br>+; CHECK: movq [[ARG2_SLOT]], %rsi<br>+; Load the thrid argument<br>+; CHECK: movq [[ARG2_SLOT]], %rdx<br>+; Load the function pointer.<br>+; CHECK: movq [[LOADED_PTR_SLOT]], [[FCT_PTR:%[a-z]+]]<br>+; Call.<br>+; CHECK: callq *[[FCT_PTR]]<br>+; CHECK: ret<br>+define i64 @test_inttoptr(i64 (i64, i64, i64)** %arg, i1 %bool, i64 %arg2) {<br>+entry:<br>+ %loaded_ptr = load i64 (i64, i64, i64)** %arg, align 8<br>+ %raw = ptrtoint i64 (i64, i64, i64)* %loaded_ptr to i64<br>+ switch i1 %bool, label %default [<br>+ i1 true, label %label_true<br>+ i1 false, label %label_end<br>+ ]<br>+default:<br>+ unreachable<br>+<br>+label_true:<br>+ br label %label_end<br>+<br>+label_end:<br>+ %fct_ptr = inttoptr i64 %raw to i64 (i64, i64, i64)*<br>+ %res = call i64 %fct_ptr(i64 %arg2, i64 %arg2, i64 %arg2)<br>+ ret i64 %res<br>+}<br>+<br>+; CHECK-LABEL: @test_ptrtoint<br>+; Load the value of the function pointer: %loaded_ptr<br>+; CHECK: movq (%rdi), [[LOADED_PTR:%[a-z]+]]<br>+; Spill %arg2.<br>+; CHECK: movq %rdx, [[ARG2_SLOT:[0-9]*\(%[a-z]+\)]]<br>+; Spill %loaded_ptr.<br>+; CHECK: movq [[LOADED_PTR]], [[LOADED_PTR_SLOT:[0-9]*\(%[a-z]+\)]]<br>+; Perform the indirect call.<br>+; Load the first argument<br>+; CHECK: movq [[ARG2_SLOT]], %rdi<br>+; Load the second argument<br>+; CHECK: movq [[ARG2_SLOT]], %rsi<br>+; Load the thrid argument<br>+; CHECK: movq [[ARG2_SLOT]], %rdx<br>+; Load the function pointer.<br>+; CHECK: movq [[LOADED_PTR_SLOT]], [[FCT_PTR:%[a-z]+]]<br>+; Call.<br>+; CHECK: callq *[[FCT_PTR]]<br>+; CHECK: ret<br>+define i64 @test_ptrtoint(i64 (i64, i64, i64)** %arg, i1 %bool, i64 %arg2) {<br>+entry:<br>+ %loaded_ptr = load i64 (i64, i64, i64)** %arg, align 8<br>+ %raw = bitcast i64 (i64, i64, i64)* %loaded_ptr to i8*<br>+ switch i1 %bool, label %default [<br>+ i1 true, label %label_true<br>+ i1 false, label %label_end<br>+ ]<br>+default:<br>+ unreachable<br>+<br>+label_true:<br>+ br label %label_end<br>+<br>+label_end:<br>+ %fct_int = ptrtoint i8* %raw to i64<br>+ %fct_ptr = inttoptr i64 %fct_int to i64 (i64, i64, i64)*<br>+ %res = call i64 %fct_ptr(i64 %arg2, i64 %arg2, i64 %arg2)<br>+ ret i64 %res<br>+}<br><br><br>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits<br></blockquote></blockquote></div><br></div></body></html>