From 32d75bec4bbbd97b70d887ed3c106951b1d18182 Mon Sep 17 00:00:00 2001 From: Chandler Carruth Date: Wed, 18 Jul 2012 18:58:22 +0000 Subject: [PATCH] Fix a somewhat nasty crasher in PR13378. This crashes inside of LiveIntervals due to the two-addr pass generating bogus MI code. The crux of the issue was a loop nesting problem. The intent of the code which attempts to transform instructions before converting them to two-addr form is to defer and reprocess any transformed instructions as the second processing is likely to have more opportunities to coalesce copies, etc. Unfortunately, there was one section of processing that was not deferred -- the INSERT_SUBREG rewriting. Due to quirks of how this rewriting proceeded, not only did it occur early, it removed the bits of information needed for the deferred processing to correctly generate the necessary two address form (specifically inserting a copy), but didn't trigger any immediate assertions and produced what appeared to be already valid two-address from code. Thus, the assertion only fired much later in the pipeline. The fix is to hoist the transformation logic up layer to where it can more firmly defer all further processing, and to teach the normal processing to handle an edge case previously handled as part of the transformation logic. This edge case (already matched tied register operands) needs to *not* defer any steps. As has been brought up repeatedly in the process: wow does this code need refactoring. I *may* squeeze in some time to at least bring sanity to this loop... but wow... =] Thanks to Jakob for helpful hints on the way here, and the review. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@160443 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/TwoAddressInstructionPass.cpp | 54 ++++++++++++++--------- test/CodeGen/ARM/twoaddrinstr.ll | 21 +++++++++ 2 files changed, 53 insertions(+), 22 deletions(-) create mode 100644 test/CodeGen/ARM/twoaddrinstr.ll diff --git a/lib/CodeGen/TwoAddressInstructionPass.cpp b/lib/CodeGen/TwoAddressInstructionPass.cpp index 196e06ea519..e4c011901ed 100644 --- a/lib/CodeGen/TwoAddressInstructionPass.cpp +++ b/lib/CodeGen/TwoAddressInstructionPass.cpp @@ -1451,28 +1451,34 @@ bool TwoAddressInstructionPass::runOnMachineFunction(MachineFunction &MF) { TiedOperands[regB].push_back(std::make_pair(SrcIdx, DstIdx)); } + // If the instruction has a single pair of tied operands, try some + // transformations that may either eliminate the tied operands or + // improve the opportunities for coalescing away the register copy. + if (TiedOperands.size() == 1) { + SmallVector, 4> &TiedPairs + = TiedOperands.begin()->second; + if (TiedPairs.size() == 1) { + unsigned SrcIdx = TiedPairs[0].first; + unsigned DstIdx = TiedPairs[0].second; + unsigned SrcReg = mi->getOperand(SrcIdx).getReg(); + unsigned DstReg = mi->getOperand(DstIdx).getReg(); + if (SrcReg != DstReg && + TryInstructionTransform(mi, nmi, mbbi, SrcIdx, DstIdx, Dist, + Processed)) { + // The tied operands have been eliminated or shifted further down the + // block to ease elimination. Continue processing with 'nmi'. + TiedOperands.clear(); + mi = nmi; + continue; + } + } + } + // Now iterate over the information collected above. for (TiedOperandMap::iterator OI = TiedOperands.begin(), OE = TiedOperands.end(); OI != OE; ++OI) { SmallVector, 4> &TiedPairs = OI->second; - // If the instruction has a single pair of tied operands, try some - // transformations that may either eliminate the tied operands or - // improve the opportunities for coalescing away the register copy. - if (TiedOperands.size() == 1 && TiedPairs.size() == 1) { - unsigned SrcIdx = TiedPairs[0].first; - unsigned DstIdx = TiedPairs[0].second; - - // If the registers are already equal, nothing needs to be done. - if (mi->getOperand(SrcIdx).getReg() == - mi->getOperand(DstIdx).getReg()) - break; // Done with this instruction. - - if (TryInstructionTransform(mi, nmi, mbbi, SrcIdx, DstIdx, Dist, - Processed)) - break; // The tied operands have been eliminated. - } - bool IsEarlyClobber = false; bool RemovedKillFlag = false; bool AllUsesCopied = true; @@ -1593,12 +1599,16 @@ bool TwoAddressInstructionPass::runOnMachineFunction(MachineFunction &MF) { } } - // Schedule the source copy / remat inserted to form two-address - // instruction. FIXME: Does it matter the distance map may not be - // accurate after it's scheduled? - TII->scheduleTwoAddrSource(prior(mi), mi, *TRI); + // We didn't change anything if there was a single tied pair, and that + // pair didn't require copies. + if (AllUsesCopied || TiedPairs.size() > 1) { + MadeChange = true; - MadeChange = true; + // Schedule the source copy / remat inserted to form two-address + // instruction. FIXME: Does it matter the distance map may not be + // accurate after it's scheduled? + TII->scheduleTwoAddrSource(prior(mi), mi, *TRI); + } DEBUG(dbgs() << "\t\trewrite to:\t" << *mi); } diff --git a/test/CodeGen/ARM/twoaddrinstr.ll b/test/CodeGen/ARM/twoaddrinstr.ll new file mode 100644 index 00000000000..4e227dd5be3 --- /dev/null +++ b/test/CodeGen/ARM/twoaddrinstr.ll @@ -0,0 +1,21 @@ +; Tests for the two-address instruction pass. +; RUN: llc -march=arm -mcpu=cortex-a9 < %s | FileCheck %s + +define void @PR13378() nounwind { +; This was orriginally a crasher trying to schedule the instructions. +; CHECK: PR13378: +; CHECK: vldmia +; CHECK-NEXT: vmov.f32 +; CHECK-NEXT: vstmia +; CHECK-NEXT: vstmia +; CHECK-NEXT: vmov.f32 +; CHECK-NEXT: vstmia + +entry: + %0 = load <4 x float>* undef + store <4 x float> zeroinitializer, <4 x float>* undef + store <4 x float> %0, <4 x float>* undef + %1 = insertelement <4 x float> %0, float 1.000000e+00, i32 3 + store <4 x float> %1, <4 x float>* undef + unreachable +} -- 2.34.1