mirror of
https://github.com/c64scene-ar/llvm-6502.git
synced 2024-08-30 21:29:28 +00:00
Fix the time regression I introduced in 464.h264ref with
my last patch to this file. The issue there was that all uses of an IV inside a loop are actually references to Base[IV*2], and there was one use outside that was the same but LSR didn't see the base or the scaling because it didn't recurse into uses outside the loop; thus, it used base+IV*scale mode inside the loop instead of pulling base out of the loop. This was extra bad because register pressure later forced both base and IV into memory. Doing that recursion, at least enough to figure out addressing modes, is a good idea in general; the change in AddUsersIfInteresting does this. However, there were side effects.... It is also possible for recursing outside the loop to introduce another IV where there was only 1 before (if the refs inside are not scaled and the ref outside is). I don't think this is a common case, but it's in the testsuite. It is right to be very aggressive about getting rid of such introduced IVs (CheckForIVReuse and the handling of nonzero RewriteFactor in StrengthReduceStridedIVUsers). In the testcase in question the new IV produced this way has both a nonconstant stride and a nonzero base, neither of which was handled before. (This patch does not handle all the cases where this can happen.) And when inserting new code that feeds into a PHI, it's right to put such code at the original location rather than in the PHI's immediate predecessor(s) when the original location is outside the loop (a case that couldn't happen before) (RewriteInstructionToUseNewBase); better to avoid making multiple copies of it in this case. Everything above is exercised in CodeGen/X86/lsr-negative-stride.ll (and ifcvt4 in ARM which is the same IR). git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@61178 91177308-0d34-0410-b5e6-96231b3b80d8
This commit is contained in:
parent
5f4f84ba3e
commit
12b882cf53
@ -508,14 +508,22 @@ bool LoopStrengthReduce::AddUsersIfInteresting(Instruction *I, Loop *L,
|
|||||||
if (isa<PHINode>(User) && Processed.count(User))
|
if (isa<PHINode>(User) && Processed.count(User))
|
||||||
continue;
|
continue;
|
||||||
|
|
||||||
// If this is an instruction defined in a nested loop, or outside this loop,
|
// Descend recursively, but not into PHI nodes outside the current loop.
|
||||||
// don't recurse into it.
|
// It's important to see the entire expression outside the loop to get
|
||||||
|
// choices that depend on addressing mode use right, although we won't
|
||||||
|
// consider references ouside the loop in all cases.
|
||||||
|
// If User is already in Processed, we don't want to recurse into it again,
|
||||||
|
// but do want to record a second reference in the same instruction.
|
||||||
bool AddUserToIVUsers = false;
|
bool AddUserToIVUsers = false;
|
||||||
if (LI->getLoopFor(User->getParent()) != L) {
|
if (LI->getLoopFor(User->getParent()) != L) {
|
||||||
DOUT << "FOUND USER in other loop: " << *User
|
if (isa<PHINode>(User) || Processed.count(User) ||
|
||||||
<< " OF SCEV: " << *ISE << "\n";
|
!AddUsersIfInteresting(User, L, Processed)) {
|
||||||
AddUserToIVUsers = true;
|
DOUT << "FOUND USER in other loop: " << *User
|
||||||
} else if (!AddUsersIfInteresting(User, L, Processed)) {
|
<< " OF SCEV: " << *ISE << "\n";
|
||||||
|
AddUserToIVUsers = true;
|
||||||
|
}
|
||||||
|
} else if (Processed.count(User) ||
|
||||||
|
!AddUsersIfInteresting(User, L, Processed)) {
|
||||||
DOUT << "FOUND USER: " << *User
|
DOUT << "FOUND USER: " << *User
|
||||||
<< " OF SCEV: " << *ISE << "\n";
|
<< " OF SCEV: " << *ISE << "\n";
|
||||||
AddUserToIVUsers = true;
|
AddUserToIVUsers = true;
|
||||||
@ -704,34 +712,45 @@ void BasedUser::RewriteInstructionToUseNewBase(const SCEVHandle &NewBase,
|
|||||||
PHINode *PN = cast<PHINode>(Inst);
|
PHINode *PN = cast<PHINode>(Inst);
|
||||||
for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
|
for (unsigned i = 0, e = PN->getNumIncomingValues(); i != e; ++i) {
|
||||||
if (PN->getIncomingValue(i) == OperandValToReplace) {
|
if (PN->getIncomingValue(i) == OperandValToReplace) {
|
||||||
// If this is a critical edge, split the edge so that we do not insert the
|
// If the original expression is outside the loop, put the replacement
|
||||||
// code on all predecessor/successor paths. We do this unless this is the
|
// code in the same place as the original expression,
|
||||||
// canonical backedge for this loop, as this can make some inserted code
|
// which need not be an immediate predecessor of this PHI. This way we
|
||||||
// be in an illegal position.
|
// need only one copy of it even if it is referenced multiple times in
|
||||||
BasicBlock *PHIPred = PN->getIncomingBlock(i);
|
// the PHI. We don't do this when the original expression is inside the
|
||||||
if (e != 1 && PHIPred->getTerminator()->getNumSuccessors() > 1 &&
|
// loop because multiple copies sometimes do useful sinking of code in that
|
||||||
(PN->getParent() != L->getHeader() || !L->contains(PHIPred))) {
|
// case(?).
|
||||||
|
Instruction *OldLoc = dyn_cast<Instruction>(OperandValToReplace);
|
||||||
// First step, split the critical edge.
|
if (L->contains(OldLoc->getParent())) {
|
||||||
SplitCriticalEdge(PHIPred, PN->getParent(), P, false);
|
// If this is a critical edge, split the edge so that we do not insert the
|
||||||
|
// code on all predecessor/successor paths. We do this unless this is the
|
||||||
// Next step: move the basic block. In particular, if the PHI node
|
// canonical backedge for this loop, as this can make some inserted code
|
||||||
// is outside of the loop, and PredTI is in the loop, we want to
|
// be in an illegal position.
|
||||||
// move the block to be immediately before the PHI block, not
|
BasicBlock *PHIPred = PN->getIncomingBlock(i);
|
||||||
// immediately after PredTI.
|
if (e != 1 && PHIPred->getTerminator()->getNumSuccessors() > 1 &&
|
||||||
if (L->contains(PHIPred) && !L->contains(PN->getParent())) {
|
(PN->getParent() != L->getHeader() || !L->contains(PHIPred))) {
|
||||||
BasicBlock *NewBB = PN->getIncomingBlock(i);
|
|
||||||
NewBB->moveBefore(PN->getParent());
|
|
||||||
}
|
|
||||||
|
|
||||||
// Splitting the edge can reduce the number of PHI entries we have.
|
|
||||||
e = PN->getNumIncomingValues();
|
|
||||||
}
|
|
||||||
|
|
||||||
|
// First step, split the critical edge.
|
||||||
|
SplitCriticalEdge(PHIPred, PN->getParent(), P, false);
|
||||||
|
|
||||||
|
// Next step: move the basic block. In particular, if the PHI node
|
||||||
|
// is outside of the loop, and PredTI is in the loop, we want to
|
||||||
|
// move the block to be immediately before the PHI block, not
|
||||||
|
// immediately after PredTI.
|
||||||
|
if (L->contains(PHIPred) && !L->contains(PN->getParent())) {
|
||||||
|
BasicBlock *NewBB = PN->getIncomingBlock(i);
|
||||||
|
NewBB->moveBefore(PN->getParent());
|
||||||
|
}
|
||||||
|
|
||||||
|
// Splitting the edge can reduce the number of PHI entries we have.
|
||||||
|
e = PN->getNumIncomingValues();
|
||||||
|
}
|
||||||
|
}
|
||||||
Value *&Code = InsertedCode[PN->getIncomingBlock(i)];
|
Value *&Code = InsertedCode[PN->getIncomingBlock(i)];
|
||||||
if (!Code) {
|
if (!Code) {
|
||||||
// Insert the code into the end of the predecessor block.
|
// Insert the code into the end of the predecessor block.
|
||||||
Instruction *InsertPt = PN->getIncomingBlock(i)->getTerminator();
|
Instruction *InsertPt = (L->contains(OldLoc->getParent())) ?
|
||||||
|
PN->getIncomingBlock(i)->getTerminator() :
|
||||||
|
OldLoc->getParent()->getTerminator();
|
||||||
Code = InsertCodeForBaseAtPosition(NewBase, Rewriter, InsertPt, L);
|
Code = InsertCodeForBaseAtPosition(NewBase, Rewriter, InsertPt, L);
|
||||||
|
|
||||||
// Adjust the type back to match the PHI. Note that we can't use
|
// Adjust the type back to match the PHI. Note that we can't use
|
||||||
@ -1168,6 +1187,10 @@ bool LoopStrengthReduce::RequiresTypeConversion(const Type *Ty1,
|
|||||||
/// mode scale component and optional base reg. This allows the users of
|
/// mode scale component and optional base reg. This allows the users of
|
||||||
/// this stride to be rewritten as prev iv * factor. It returns 0 if no
|
/// this stride to be rewritten as prev iv * factor. It returns 0 if no
|
||||||
/// reuse is possible. Factors can be negative on same targets, e.g. ARM.
|
/// reuse is possible. Factors can be negative on same targets, e.g. ARM.
|
||||||
|
///
|
||||||
|
/// If all uses are outside the loop, we don't require that all multiplies
|
||||||
|
/// be folded into the addressing mode; a multiply (executed once) outside
|
||||||
|
/// the loop is better than another IV within. Well, usually.
|
||||||
int64_t LoopStrengthReduce::CheckForIVReuse(bool HasBaseReg,
|
int64_t LoopStrengthReduce::CheckForIVReuse(bool HasBaseReg,
|
||||||
bool AllUsesAreAddresses,
|
bool AllUsesAreAddresses,
|
||||||
bool AllUsesAreOutsideLoop,
|
bool AllUsesAreOutsideLoop,
|
||||||
@ -1205,6 +1228,30 @@ int64_t LoopStrengthReduce::CheckForIVReuse(bool HasBaseReg,
|
|||||||
return Scale;
|
return Scale;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
} else if (AllUsesAreOutsideLoop) {
|
||||||
|
// Accept nonconstant strides here; it is really really right to substitute
|
||||||
|
// an existing IV if we can.
|
||||||
|
// Special case, old IV is -1*x and this one is x. Can treat this one as
|
||||||
|
// -1*old.
|
||||||
|
for (unsigned NewStride = 0, e = StrideOrder.size(); NewStride != e;
|
||||||
|
++NewStride) {
|
||||||
|
std::map<SCEVHandle, IVsOfOneStride>::iterator SI =
|
||||||
|
IVsByStride.find(StrideOrder[NewStride]);
|
||||||
|
if (SI == IVsByStride.end())
|
||||||
|
continue;
|
||||||
|
if (SCEVMulExpr *ME = dyn_cast<SCEVMulExpr>(SI->first))
|
||||||
|
if (SCEVConstant *SC = dyn_cast<SCEVConstant>(ME->getOperand(0)))
|
||||||
|
if (Stride == ME->getOperand(1) &&
|
||||||
|
SC->getValue()->getSExtValue() == -1LL)
|
||||||
|
for (std::vector<IVExpr>::iterator II = SI->second.IVs.begin(),
|
||||||
|
IE = SI->second.IVs.end(); II != IE; ++II)
|
||||||
|
// Accept nonzero base here.
|
||||||
|
// Only reuse previous IV if it would not require a type conversion.
|
||||||
|
if (!RequiresTypeConversion(II->Base->getType(), Ty)) {
|
||||||
|
IV = *II;
|
||||||
|
return -1;
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return 0;
|
return 0;
|
||||||
}
|
}
|
||||||
@ -1549,6 +1596,18 @@ void LoopStrengthReduce::StrengthReduceStridedIVUsers(const SCEVHandle &Stride,
|
|||||||
!cast<ConstantInt>(CommonBaseV)->isZero())
|
!cast<ConstantInt>(CommonBaseV)->isZero())
|
||||||
RewriteExpr = SE->getAddExpr(RewriteExpr,
|
RewriteExpr = SE->getAddExpr(RewriteExpr,
|
||||||
SE->getUnknown(CommonBaseV));
|
SE->getUnknown(CommonBaseV));
|
||||||
|
// If we're reusing an IV with a nonzero base (currently this happens
|
||||||
|
// only when all reuses are outside the loop) subtract out that base here
|
||||||
|
// This is the reverse of the above; the base HAS been used to initialize
|
||||||
|
// the PHI node but we don't want it here.
|
||||||
|
// (If the RewriteFactor is negative, we're effectively negating the
|
||||||
|
// old IV in this use, so we add the base instead of subtract.)
|
||||||
|
if (!ReuseIV.Base->isZero()) {
|
||||||
|
if (RewriteFactor < 0)
|
||||||
|
RewriteExpr = SE->getAddExpr(RewriteExpr, ReuseIV.Base);
|
||||||
|
else
|
||||||
|
RewriteExpr = SE->getMinusSCEV(RewriteExpr, ReuseIV.Base);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Now that we know what we need to do, insert code before User for the
|
// Now that we know what we need to do, insert code before User for the
|
||||||
|
Loading…
Reference in New Issue
Block a user