From 256a16d0314c78b0ee3612e01d0cafbf6c83de7b Mon Sep 17 00:00:00 2001 From: Tim Northover Date: Mon, 17 Dec 2018 17:25:53 +0000 Subject: [PATCH] FastIsel: take care to update iterators when removing instructions. We keep a few iterators into the basic block we're selecting while performing FastISel. Usually this is fine, but occasionally code wants to remove already-emitted instructions. When this happens we have to be careful to update those iterators so they're not pointint at dangling memory. llvm-svn: 349365 --- llvm/lib/CodeGen/SelectionDAG/FastISel.cpp | 9 +++++++++ llvm/lib/Target/AArch64/AArch64FastISel.cpp | 11 +++++++---- llvm/lib/Target/ARM/ARMFastISel.cpp | 3 ++- llvm/lib/Target/PowerPC/PPCFastISel.cpp | 3 ++- llvm/lib/Target/X86/X86FastISel.cpp | 3 ++- llvm/test/CodeGen/AArch64/fast-isel-erase.ll | 25 +++++++++++++++++++++++++ 6 files changed, 47 insertions(+), 7 deletions(-) create mode 100644 llvm/test/CodeGen/AArch64/fast-isel-erase.ll diff --git a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp index d5f066c2..a9a3c44 100644 --- a/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp +++ b/llvm/lib/CodeGen/SelectionDAG/FastISel.cpp @@ -547,6 +547,15 @@ void FastISel::removeDeadCode(MachineBasicBlock::iterator I, assert(I.isValid() && E.isValid() && std::distance(I, E) > 0 && "Invalid iterator!"); while (I != E) { + if (LastFlushPoint == I) + LastFlushPoint = E; + if (SavedInsertPt == I) + SavedInsertPt = E; + if (EmitStartPt == I) + EmitStartPt = E.isValid() ? &*E : nullptr; + if (LastLocalValue == I) + LastLocalValue = E.isValid() ? &*E : nullptr; + MachineInstr *Dead = &*I; ++I; Dead->eraseFromParent(); diff --git a/llvm/lib/Target/AArch64/AArch64FastISel.cpp b/llvm/lib/Target/AArch64/AArch64FastISel.cpp index dfc08a1..7a7b0dd 100644 --- a/llvm/lib/Target/AArch64/AArch64FastISel.cpp +++ b/llvm/lib/Target/AArch64/AArch64FastISel.cpp @@ -2016,8 +2016,9 @@ bool AArch64FastISel::selectLoad(const Instruction *I) { if (RetVT == MVT::i64 && VT <= MVT::i32) { if (WantZExt) { // Delete the last emitted instruction from emitLoad (SUBREG_TO_REG). - std::prev(FuncInfo.InsertPt)->eraseFromParent(); - ResultReg = std::prev(FuncInfo.InsertPt)->getOperand(0).getReg(); + MachineBasicBlock::iterator I(std::prev(FuncInfo.InsertPt)); + ResultReg = std::prev(I)->getOperand(0).getReg(); + removeDeadCode(I, std::next(I)); } else ResultReg = fastEmitInst_extractsubreg(MVT::i32, ResultReg, /*IsKill=*/true, @@ -2038,7 +2039,8 @@ bool AArch64FastISel::selectLoad(const Instruction *I) { break; } } - MI->eraseFromParent(); + MachineBasicBlock::iterator I(MI); + removeDeadCode(I, std::next(I)); MI = nullptr; if (Reg) MI = MRI.getUniqueVRegDef(Reg); @@ -4508,7 +4510,8 @@ bool AArch64FastISel::optimizeIntExtLoad(const Instruction *I, MVT RetVT, MI->getOperand(1).getSubReg() == AArch64::sub_32) && "Expected copy instruction"); Reg = MI->getOperand(1).getReg(); - MI->eraseFromParent(); + MachineBasicBlock::iterator I(MI); + removeDeadCode(I, std::next(I)); } updateValueMap(I, Reg); return true; diff --git a/llvm/lib/Target/ARM/ARMFastISel.cpp b/llvm/lib/Target/ARM/ARMFastISel.cpp index fd3d10a..a50abfd 100644 --- a/llvm/lib/Target/ARM/ARMFastISel.cpp +++ b/llvm/lib/Target/ARM/ARMFastISel.cpp @@ -2951,7 +2951,8 @@ bool ARMFastISel::tryToFoldLoadIntoMI(MachineInstr *MI, unsigned OpNo, unsigned ResultReg = MI->getOperand(0).getReg(); if (!ARMEmitLoad(VT, ResultReg, Addr, LI->getAlignment(), isZExt, false)) return false; - MI->eraseFromParent(); + MachineBasicBlock::iterator I(MI); + removeDeadCode(I, std::next(I)); return true; } diff --git a/llvm/lib/Target/PowerPC/PPCFastISel.cpp b/llvm/lib/Target/PowerPC/PPCFastISel.cpp index 6681698..aa55ac1 100644 --- a/llvm/lib/Target/PowerPC/PPCFastISel.cpp +++ b/llvm/lib/Target/PowerPC/PPCFastISel.cpp @@ -2354,7 +2354,8 @@ bool PPCFastISel::tryToFoldLoadIntoMI(MachineInstr *MI, unsigned OpNo, PPCSubTarget->hasSPE() ? PPC::EVLDD : PPC::LFD)) return false; - MI->eraseFromParent(); + MachineBasicBlock::iterator I(MI); + removeDeadCode(I, std::next(I)); return true; } diff --git a/llvm/lib/Target/X86/X86FastISel.cpp b/llvm/lib/Target/X86/X86FastISel.cpp index a49ad8b..cbfdc4b 100644 --- a/llvm/lib/Target/X86/X86FastISel.cpp +++ b/llvm/lib/Target/X86/X86FastISel.cpp @@ -3998,7 +3998,8 @@ bool X86FastISel::tryToFoldLoadIntoMI(MachineInstr *MI, unsigned OpNo, } Result->addMemOperand(*FuncInfo.MF, createMachineMemOperandFor(LI)); - MI->eraseFromParent(); + MachineBasicBlock::iterator I(MI); + removeDeadCode(I, std::next(I)); return true; } diff --git a/llvm/test/CodeGen/AArch64/fast-isel-erase.ll b/llvm/test/CodeGen/AArch64/fast-isel-erase.ll new file mode 100644 index 0000000..e8265bc --- /dev/null +++ b/llvm/test/CodeGen/AArch64/fast-isel-erase.ll @@ -0,0 +1,25 @@ +; RUN: llc -mtriple=arm64-apple-ios -o - %s -fast-isel=1 -O0 | FileCheck %s + +; The zext can be folded into the load and removed, but doing so can invalidate +; pointers internal to FastISel and cause a crash so it must be done carefully. +define i32 @test() { +; CHECK-LABEL: test: +; CHECK: ldrh +; CHECK: bl _callee +; CHECK-NOT: uxth + +entry: + store i32 undef, i32* undef, align 4 + %t81 = load i16, i16* undef, align 2 + call void @callee() + %t82 = zext i16 %t81 to i32 + %t83 = shl i32 %t82, 16 + %t84 = or i32 undef, %t83 + br label %end + +end: + %val = phi i32 [%t84, %entry] + ret i32 %val +} + +declare void @callee() -- 2.7.4