From c86f47bee2d09d17e008fbeabb6a7c55005ccc65 Mon Sep 17 00:00:00 2001 From: Arnold Schwaighofer Date: Thu, 15 Jun 2017 17:34:42 +0000 Subject: [PATCH] ISel: Fix FastISel of swifterror values The code assumed that we process instructions in basic block order. FastISel processes instructions in reverse basic block order. We need to pre-assign virtual registers before selecting otherwise we get def-use relationships wrong. This only affects code with swifterror registers. rdar://32659327 llvm-svn: 305484 --- include/llvm/CodeGen/FunctionLoweringInfo.h | 12 ++ .../SelectionDAG/FunctionLoweringInfo.cpp | 26 +++++ .../SelectionDAG/SelectionDAGBuilder.cpp | 34 +++--- lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp | 79 +++++++++++++ test/CodeGen/AArch64/swifterror.ll | 27 +++++ test/CodeGen/ARM/swifterror.ll | 28 +++++ test/CodeGen/X86/swifterror.ll | 108 ++++++++++++++++++ 7 files changed, 300 insertions(+), 14 deletions(-) diff --git a/include/llvm/CodeGen/FunctionLoweringInfo.h b/include/llvm/CodeGen/FunctionLoweringInfo.h index 7d7c3e8cfd2..f32a5891511 100644 --- a/include/llvm/CodeGen/FunctionLoweringInfo.h +++ b/include/llvm/CodeGen/FunctionLoweringInfo.h @@ -82,6 +82,11 @@ public: DenseMap, unsigned> SwiftErrorVRegUpwardsUse; + /// A map from instructions that define/use a swifterror value to the virtual + /// register that represents that def/use. + llvm::DenseMap, unsigned> + SwiftErrorVRegDefUses; + /// The swifterror argument of the current function. const Value *SwiftErrorArg; @@ -101,6 +106,13 @@ public: void setCurrentSwiftErrorVReg(const MachineBasicBlock *MBB, const Value *, unsigned); + /// Get or create the swifterror value virtual register for a def of a + /// swifterror by an instruction. + std::pair getOrCreateSwiftErrorVRegDefAt(const Instruction *); + std::pair + getOrCreateSwiftErrorVRegUseAt(const Instruction *, const MachineBasicBlock *, + const Value *); + /// ValueMap - Since we emit code for the function a basic block at a time, /// we must remember which virtual registers hold the values for /// cross-basic-block values. diff --git a/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp b/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp index 606b8952f3c..b736037d71d 100644 --- a/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp +++ b/lib/CodeGen/SelectionDAG/FunctionLoweringInfo.cpp @@ -523,3 +523,29 @@ void FunctionLoweringInfo::setCurrentSwiftErrorVReg( const MachineBasicBlock *MBB, const Value *Val, unsigned VReg) { SwiftErrorVRegDefMap[std::make_pair(MBB, Val)] = VReg; } + +std::pair +FunctionLoweringInfo::getOrCreateSwiftErrorVRegDefAt(const Instruction *I) { + auto Key = PointerIntPair(I, true); + auto It = SwiftErrorVRegDefUses.find(Key); + if (It == SwiftErrorVRegDefUses.end()) { + auto &DL = MF->getDataLayout(); + const TargetRegisterClass *RC = TLI->getRegClassFor(TLI->getPointerTy(DL)); + unsigned VReg = MF->getRegInfo().createVirtualRegister(RC); + SwiftErrorVRegDefUses[Key] = VReg; + return std::make_pair(VReg, true); + } + return std::make_pair(It->second, false); +} + +std::pair +FunctionLoweringInfo::getOrCreateSwiftErrorVRegUseAt(const Instruction *I, const MachineBasicBlock *MBB, const Value *Val) { + auto Key = PointerIntPair(I, false); + auto It = SwiftErrorVRegDefUses.find(Key); + if (It == SwiftErrorVRegDefUses.end()) { + unsigned VReg = getOrCreateSwiftErrorVReg(MBB, Val); + SwiftErrorVRegDefUses[Key] = VReg; + return std::make_pair(VReg, true); + } + return std::make_pair(It->second, false); +} diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp index d34ac40b949..28e0de0b114 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp +++ b/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp @@ -1496,9 +1496,10 @@ void SelectionDAGBuilder::visitRet(const ReturnInst &I) { true /*isfixed*/, 1 /*origidx*/, 0 /*partOffs*/)); // Create SDNode for the swifterror virtual register. - OutVals.push_back(DAG.getRegister(FuncInfo.getOrCreateSwiftErrorVReg( - FuncInfo.MBB, FuncInfo.SwiftErrorArg), - EVT(TLI.getPointerTy(DL)))); + OutVals.push_back( + DAG.getRegister(FuncInfo.getOrCreateSwiftErrorVRegUseAt( + &I, FuncInfo.MBB, FuncInfo.SwiftErrorArg).first, + EVT(TLI.getPointerTy(DL)))); } bool isVarArg = DAG.getMachineFunction().getFunction()->isVarArg(); @@ -3595,15 +3596,15 @@ void SelectionDAGBuilder::visitStoreToSwiftError(const StoreInst &I) { SDValue Src = getValue(SrcV); // Create a virtual register, then update the virtual register. - auto &DL = DAG.getDataLayout(); - const TargetRegisterClass *RC = TLI.getRegClassFor(TLI.getPointerTy(DL)); - unsigned VReg = FuncInfo.MF->getRegInfo().createVirtualRegister(RC); + unsigned VReg; bool CreatedVReg; + std::tie(VReg, CreatedVReg) = FuncInfo.getOrCreateSwiftErrorVRegDefAt(&I); // Chain, DL, Reg, N or Chain, DL, Reg, N, Glue // Chain can be getRoot or getControlRoot. SDValue CopyNode = DAG.getCopyToReg(getRoot(), getCurSDLoc(), VReg, SDValue(Src.getNode(), Src.getResNo())); DAG.setRoot(CopyNode); - FuncInfo.setCurrentSwiftErrorVReg(FuncInfo.MBB, I.getOperand(1), VReg); + if (CreatedVReg) + FuncInfo.setCurrentSwiftErrorVReg(FuncInfo.MBB, I.getOperand(1), VReg); } void SelectionDAGBuilder::visitLoadFromSwiftError(const LoadInst &I) { @@ -3633,7 +3634,8 @@ void SelectionDAGBuilder::visitLoadFromSwiftError(const LoadInst &I) { // Chain, DL, Reg, VT, Glue or Chain, DL, Reg, VT SDValue L = DAG.getCopyFromReg( getRoot(), getCurSDLoc(), - FuncInfo.getOrCreateSwiftErrorVReg(FuncInfo.MBB, SV), ValueVTs[0]); + FuncInfo.getOrCreateSwiftErrorVRegUseAt(&I, FuncInfo.MBB, SV).first, + ValueVTs[0]); setValue(&I, L); } @@ -6030,9 +6032,11 @@ void SelectionDAGBuilder::LowerCallTo(ImmutableCallSite CS, SDValue Callee, SwiftErrorVal = V; // We find the virtual register for the actual swifterror argument. // Instead of using the Value, we use the virtual register instead. - Entry.Node = - DAG.getRegister(FuncInfo.getOrCreateSwiftErrorVReg(FuncInfo.MBB, V), - EVT(TLI.getPointerTy(DL))); + Entry.Node = DAG.getRegister(FuncInfo + .getOrCreateSwiftErrorVRegUseAt( + CS.getInstruction(), FuncInfo.MBB, V) + .first, + EVT(TLI.getPointerTy(DL))); } Args.push_back(Entry); @@ -6073,11 +6077,13 @@ void SelectionDAGBuilder::LowerCallTo(ImmutableCallSite CS, SDValue Callee, if (SwiftErrorVal && TLI.supportSwiftError()) { // Get the last element of InVals. SDValue Src = CLI.InVals.back(); - const TargetRegisterClass *RC = TLI.getRegClassFor(TLI.getPointerTy(DL)); - unsigned VReg = FuncInfo.MF->getRegInfo().createVirtualRegister(RC); + unsigned VReg; bool CreatedVReg; + std::tie(VReg, CreatedVReg) = + FuncInfo.getOrCreateSwiftErrorVRegDefAt(CS.getInstruction()); SDValue CopyNode = CLI.DAG.getCopyToReg(Result.second, CLI.DL, VReg, Src); // We update the virtual register for the actual swifterror argument. - FuncInfo.setCurrentSwiftErrorVReg(FuncInfo.MBB, SwiftErrorVal, VReg); + if (CreatedVReg) + FuncInfo.setCurrentSwiftErrorVReg(FuncInfo.MBB, SwiftErrorVal, VReg); DAG.setRoot(CopyNode); } } diff --git a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp index b67f11f85b7..dcccd17bb98 100644 --- a/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp +++ b/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp @@ -1055,6 +1055,7 @@ static void setupSwiftErrorVals(const Function &Fn, const TargetLowering *TLI, FuncInfo->SwiftErrorVals.clear(); FuncInfo->SwiftErrorVRegDefMap.clear(); FuncInfo->SwiftErrorVRegUpwardsUse.clear(); + FuncInfo->SwiftErrorVRegDefUses.clear(); FuncInfo->SwiftErrorArg = nullptr; // Check if function has a swifterror argument. @@ -1278,6 +1279,80 @@ static void propagateSwiftErrorVRegs(FunctionLoweringInfo *FuncInfo) { } } +void preassignSwiftErrorRegs(const TargetLowering *TLI, + FunctionLoweringInfo *FuncInfo, + BasicBlock::const_iterator Begin, + BasicBlock::const_iterator End) { + if (!TLI->supportSwiftError() || FuncInfo->SwiftErrorVals.empty()) + return; + + // Iterator over instructions and assign vregs to swifterror defs and uses. + for (auto It = Begin; It != End; ++It) { + ImmutableCallSite CS(&*It); + if (CS) { + // A call-site with a swifterror argument is both use and def. + const Value *SwiftErrorAddr = nullptr; + for (auto &Arg : CS.args()) { + if (!Arg->isSwiftError()) + continue; + // Use of swifterror. + assert(!SwiftErrorAddr && "Cannot have multiple swifterror arguments"); + SwiftErrorAddr = &*Arg; + assert(SwiftErrorAddr->isSwiftError() && + "Must have a swifterror value argument"); + unsigned VReg; bool CreatedReg; + std::tie(VReg, CreatedReg) = FuncInfo->getOrCreateSwiftErrorVRegUseAt( + &*It, FuncInfo->MBB, SwiftErrorAddr); + assert(CreatedReg); + } + if (!SwiftErrorAddr) + continue; + + // Def of swifterror. + unsigned VReg; bool CreatedReg; + std::tie(VReg, CreatedReg) = + FuncInfo->getOrCreateSwiftErrorVRegDefAt(&*It); + assert(CreatedReg); + FuncInfo->setCurrentSwiftErrorVReg(FuncInfo->MBB, SwiftErrorAddr, VReg); + + // A load is a use. + } else if (const LoadInst *LI = dyn_cast(&*It)) { + const Value *V = LI->getOperand(0); + if (!V->isSwiftError()) + continue; + + unsigned VReg; bool CreatedReg; + std::tie(VReg, CreatedReg) = + FuncInfo->getOrCreateSwiftErrorVRegUseAt(LI, FuncInfo->MBB, V); + assert(CreatedReg); + + // A store is a def. + } else if (const StoreInst *SI = dyn_cast(&*It)) { + const Value *SwiftErrorAddr = SI->getOperand(1); + if (!SwiftErrorAddr->isSwiftError()) + continue; + + // Def of swifterror. + unsigned VReg; bool CreatedReg; + std::tie(VReg, CreatedReg) = + FuncInfo->getOrCreateSwiftErrorVRegDefAt(&*It); + assert(CreatedReg); + FuncInfo->setCurrentSwiftErrorVReg(FuncInfo->MBB, SwiftErrorAddr, VReg); + + // A return in a swiferror returning function is a use. + } else if (const ReturnInst *R = dyn_cast(&*It)) { + const Function *F = R->getParent()->getParent(); + if(!F->getAttributes().hasAttrSomewhere(Attribute::SwiftError)) + continue; + + unsigned VReg; bool CreatedReg; + std::tie(VReg, CreatedReg) = FuncInfo->getOrCreateSwiftErrorVRegUseAt( + R, FuncInfo->MBB, FuncInfo->SwiftErrorArg); + assert(CreatedReg); + } + } +} + void SelectionDAGISel::SelectAllBasicBlocks(const Function &Fn) { FastISelFailed = false; // Initialize the Fast-ISel state, if needed. @@ -1384,6 +1459,10 @@ void SelectionDAGISel::SelectAllBasicBlocks(const Function &Fn) { FastIS->startNewBlock(); unsigned NumFastIselRemaining = std::distance(Begin, End); + + // Pre-assign swifterror vregs. + preassignSwiftErrorRegs(TLI, FuncInfo, Begin, End); + // Do FastISel on as many instructions as possible. for (; BI != Begin; --BI) { const Instruction *Inst = &*std::prev(BI); diff --git a/test/CodeGen/AArch64/swifterror.ll b/test/CodeGen/AArch64/swifterror.ll index 69bf3510cc5..bc28f477c81 100644 --- a/test/CodeGen/AArch64/swifterror.ll +++ b/test/CodeGen/AArch64/swifterror.ll @@ -597,3 +597,30 @@ entry: tail call void @acallee(i8* null) ret void } + +declare swiftcc void @foo2(%swift_error** swifterror) + +; Make sure we properly assign registers during fast-isel. +; CHECK-O0-LABEL: testAssign +; CHECK-O0: mov [[TMP:x.*]], xzr +; CHECK-O0: mov x21, [[TMP]] +; CHECK-O0: bl _foo2 +; CHECK-O0: str x21, [s[[STK:.*]]] +; CHECK-O0: ldr x0, [s[[STK]]] + +; CHECK-APPLE-LABEL: testAssign +; CHECK-APPLE: mov x21, xzr +; CHECK-APPLE: bl _foo2 +; CHECK-APPLE: mov x0, x21 + +define swiftcc %swift_error* @testAssign(i8* %error_ref) { +entry: + %error_ptr = alloca swifterror %swift_error* + store %swift_error* null, %swift_error** %error_ptr + call swiftcc void @foo2(%swift_error** swifterror %error_ptr) + br label %a + +a: + %error = load %swift_error*, %swift_error** %error_ptr + ret %swift_error* %error +} diff --git a/test/CodeGen/ARM/swifterror.ll b/test/CodeGen/ARM/swifterror.ll index 78764202f62..3fd57c592bf 100644 --- a/test/CodeGen/ARM/swifterror.ll +++ b/test/CodeGen/ARM/swifterror.ll @@ -528,3 +528,31 @@ entry: tail call void @acallee(i8* null) ret void } + + +declare swiftcc void @foo2(%swift_error** swifterror) + +; Make sure we properly assign registers during fast-isel. +; CHECK-O0-LABEL: testAssign +; CHECK-O0: mov r8, #0 +; CHECK-O0: bl _foo2 +; CHECK-O0: str r8, [s[[STK:p.*]]] +; CHECK-O0: ldr r0, [s[[STK]]] +; CHECK-O0: pop + +; CHECK-APPLE-LABEL: testAssign +; CHECK-APPLE: mov r8, #0 +; CHECK-APPLE: bl _foo2 +; CHECK-APPLE: mov r0, r8 + +define swiftcc %swift_error* @testAssign(i8* %error_ref) { +entry: + %error_ptr = alloca swifterror %swift_error* + store %swift_error* null, %swift_error** %error_ptr + call swiftcc void @foo2(%swift_error** swifterror %error_ptr) + br label %a + +a: + %error = load %swift_error*, %swift_error** %error_ptr + ret %swift_error* %error +} diff --git a/test/CodeGen/X86/swifterror.ll b/test/CodeGen/X86/swifterror.ll index 5704d191998..1ecd33743d2 100644 --- a/test/CodeGen/X86/swifterror.ll +++ b/test/CodeGen/X86/swifterror.ll @@ -712,3 +712,111 @@ trueBB: falseBB: ret void } + + +declare swiftcc void @foo2(%swift_error** swifterror) + +; Make sure we properly assign registers during fast-isel. +; CHECK-O0-LABEL: testAssign +; CHECK-O0: pushq %r12 +; CHECK-O0: xorl [[ZERO:%[a-z0-9]+]], [[ZERO]] +; CHECK-O0: movl [[ZERO]], %r12d +; CHECK-O0: callq _foo2 +; CHECK-O0: movq %r12, [[SLOT:[-a-z0-9\(\)\%]*]] +; +; CHECK-O0: movq [[SLOT]], %rax +; CHECK-O0: popq %r12 +; CHECK-O0: retq + +; CHECK-APPLE-LABEL: testAssign +; CHECK-APPLE: pushq %r12 +; CHECK-APPLE: xorl %r12d, %r12d +; CHECK-APPLE: callq _foo2 +; CHECK-APPLE: movq %r12, %rax +; CHECK-APPLE: popq %r12 +; CHECK-APPLE: retq + +define swiftcc %swift_error* @testAssign(i8* %error_ref) { +entry: + %error_ptr = alloca swifterror %swift_error* + store %swift_error* null, %swift_error** %error_ptr + call swiftcc void @foo2(%swift_error** swifterror %error_ptr) + br label %a + +a: + %error = load %swift_error*, %swift_error** %error_ptr + ret %swift_error* %error +} + +; CHECK-O0-LABEL: testAssign2 +; CHECK-O0: movq %r12, {{.*}} +; CHECK-O0: movq %r12, [[SLOT:[-a-z0-9\(\)\%]*]] +; CHECK-O0: jmp +; CHECK-O0: movq [[SLOT]], %rax +; CHECK-O0: movq %rax, [[SLOT2:[-a-z0-9\(\)\%]*]] +; CHECK-O0: movq [[SLOT2]], %r12 +; CHECK-O0: retq + +; CHECK-APPLE-LABEL: testAssign2 +; CHECK-APPLE: movq %r12, %rax +; CHECK-APPLE: retq +define swiftcc %swift_error* @testAssign2(i8* %error_ref, %swift_error** swifterror %err) { +entry: + br label %a + +a: + %error = load %swift_error*, %swift_error** %err + ret %swift_error* %error +} + +; CHECK-O0-LABEL: testAssign3 +; CHECK-O0: callq _foo2 +; CHECK-O0: movq %r12, [[SLOT:[-a-z0-9\(\)\%]*]] +; CHECK-O0: movq [[SLOT]], %rax +; CHECK-O0: movq %rax, [[SLOT2:[-a-z0-9\(\)\%]*]] +; CHECK-O0: movq [[SLOT2]], %r12 +; CHECK-O0: addq $24, %rsp +; CHECK-O0: retq + +; CHECK-APPLE-LABEL: testAssign3 +; CHECK-APPLE: callq _foo2 +; CHECK-APPLE: movq %r12, %rax +; CHECK-APPLE: retq + +define swiftcc %swift_error* @testAssign3(i8* %error_ref, %swift_error** swifterror %err) { +entry: + call swiftcc void @foo2(%swift_error** swifterror %err) + br label %a + +a: + %error = load %swift_error*, %swift_error** %err + ret %swift_error* %error +} + + +; CHECK-O0-LABEL: testAssign4 +; CHECK-O0: callq _foo2 +; CHECK-O0: xorl %ecx, %ecx +; CHECK-O0: movl %ecx, %eax +; CHECK-O0: movq %rax, [[SLOT:[-a-z0-9\(\)\%]*]] +; CHECK-O0: movq [[SLOT]], %rax +; CHECK-O0: movq %rax, [[SLOT2:[-a-z0-9\(\)\%]*]] +; CHECK-O0: movq [[SLOT2]], %r12 +; CHECK-O0: retq + +; CHECK-APPLE-LABEL: testAssign4 +; CHECK-APPLE: callq _foo2 +; CHECK-APPLE: xorl %eax, %eax +; CHECK-APPLE: xorl %r12d, %r12d +; CHECK-APPLE: retq + +define swiftcc %swift_error* @testAssign4(i8* %error_ref, %swift_error** swifterror %err) { +entry: + call swiftcc void @foo2(%swift_error** swifterror %err) + store %swift_error* null, %swift_error** %err + br label %a + +a: + %error = load %swift_error*, %swift_error** %err + ret %swift_error* %error +}