[llvm-commits] [llvm] r160443 - in /llvm/trunk: lib/CodeGen/TwoAddressInstructionPass.cpp test/CodeGen/ARM/twoaddrinstr.ll
Chandler Carruth
chandlerc at gmail.com
Wed Jul 18 11:58:23 PDT 2012
Author: chandlerc
Date: Wed Jul 18 13:58:22 2012
New Revision: 160443
URL: http://llvm.org/viewvc/llvm-project?rev=160443&view=rev
Log:
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.
Added:
llvm/trunk/test/CodeGen/ARM/twoaddrinstr.ll
Modified:
llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp
Modified: llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp?rev=160443&r1=160442&r2=160443&view=diff
==============================================================================
--- llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp (original)
+++ llvm/trunk/lib/CodeGen/TwoAddressInstructionPass.cpp Wed Jul 18 13:58:22 2012
@@ -1451,28 +1451,34 @@
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<std::pair<unsigned, unsigned>, 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<std::pair<unsigned, unsigned>, 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 @@
}
}
- // 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);
-
- MadeChange = true;
+ // 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;
+
+ // 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);
}
Added: llvm/trunk/test/CodeGen/ARM/twoaddrinstr.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/ARM/twoaddrinstr.ll?rev=160443&view=auto
==============================================================================
--- llvm/trunk/test/CodeGen/ARM/twoaddrinstr.ll (added)
+++ llvm/trunk/test/CodeGen/ARM/twoaddrinstr.ll Wed Jul 18 13:58:22 2012
@@ -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
+}
More information about the llvm-commits
mailing list