Skip to content

Commit aa00b1d

Browse files
committed
[LV] Try to sink users recursively for first-order recurrences.
Update isFirstOrderRecurrence to explore all uses of a recurrence phi and check if we can sink them. If there are multiple users to sink, they are all mapped to the previous instruction. Fixes PR44286 (and another PR or two). Reviewed By: Ayal Differential Revision: https://reviews.llvm.org/D84951
1 parent 73b759a commit aa00b1d

File tree

7 files changed

+254
-70
lines changed

7 files changed

+254
-70
lines changed

llvm/include/llvm/Analysis/IVDescriptors.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#define LLVM_ANALYSIS_IVDESCRIPTORS_H
1515

1616
#include "llvm/ADT/DenseMap.h"
17+
#include "llvm/ADT/MapVector.h"
1718
#include "llvm/ADT/SmallPtrSet.h"
1819
#include "llvm/ADT/SmallVector.h"
1920
#include "llvm/ADT/StringRef.h"
@@ -174,7 +175,7 @@ class RecurrenceDescriptor {
174175
/// to handle Phi as a first-order recurrence.
175176
static bool
176177
isFirstOrderRecurrence(PHINode *Phi, Loop *TheLoop,
177-
DenseMap<Instruction *, Instruction *> &SinkAfter,
178+
MapVector<Instruction *, Instruction *> &SinkAfter,
178179
DominatorTree *DT);
179180

180181
RecurKind getRecurrenceKind() const { return Kind; }

llvm/include/llvm/Transforms/Vectorize/LoopVectorizationLegality.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ class LoopVectorizationLegality {
295295
RecurrenceSet &getFirstOrderRecurrences() { return FirstOrderRecurrences; }
296296

297297
/// Return the set of instructions to sink to handle first-order recurrences.
298-
DenseMap<Instruction *, Instruction *> &getSinkAfter() { return SinkAfter; }
298+
MapVector<Instruction *, Instruction *> &getSinkAfter() { return SinkAfter; }
299299

300300
/// Returns the widest induction type.
301301
Type *getWidestInductionType() { return WidestIndTy; }
@@ -518,7 +518,7 @@ class LoopVectorizationLegality {
518518

519519
/// Holds instructions that need to sink past other instructions to handle
520520
/// first-order recurrences.
521-
DenseMap<Instruction *, Instruction *> SinkAfter;
521+
MapVector<Instruction *, Instruction *> SinkAfter;
522522

523523
/// Holds the widest induction type encountered.
524524
Type *WidestIndTy = nullptr;

llvm/lib/Analysis/IVDescriptors.cpp

Lines changed: 56 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@
3434
#include "llvm/Support/Debug.h"
3535
#include "llvm/Support/KnownBits.h"
3636

37+
#include <set>
38+
3739
using namespace llvm;
3840
using namespace llvm::PatternMatch;
3941

@@ -715,7 +717,7 @@ bool RecurrenceDescriptor::isReductionPHI(PHINode *Phi, Loop *TheLoop,
715717

716718
bool RecurrenceDescriptor::isFirstOrderRecurrence(
717719
PHINode *Phi, Loop *TheLoop,
718-
DenseMap<Instruction *, Instruction *> &SinkAfter, DominatorTree *DT) {
720+
MapVector<Instruction *, Instruction *> &SinkAfter, DominatorTree *DT) {
719721

720722
// Ensure the phi node is in the loop header and has two incoming values.
721723
if (Phi->getParent() != TheLoop->getHeader() ||
@@ -741,51 +743,76 @@ bool RecurrenceDescriptor::isFirstOrderRecurrence(
741743
SinkAfter.count(Previous)) // Cannot rely on dominance due to motion.
742744
return false;
743745

744-
// Ensure every user of the phi node is dominated by the previous value.
745-
// The dominance requirement ensures the loop vectorizer will not need to
746-
// vectorize the initial value prior to the first iteration of the loop.
747-
// TODO: Consider extending this sinking to handle memory instructions and
748-
// phis with multiple users.
749-
750-
// Returns true, if all users of I are dominated by DominatedBy.
751-
auto allUsesDominatedBy = [DT](Instruction *I, Instruction *DominatedBy) {
752-
return all_of(I->uses(), [DT, DominatedBy](Use &U) {
753-
return DT->dominates(DominatedBy, U);
754-
});
746+
// Ensure every user of the phi node (recursively) is dominated by the
747+
// previous value. The dominance requirement ensures the loop vectorizer will
748+
// not need to vectorize the initial value prior to the first iteration of the
749+
// loop.
750+
// TODO: Consider extending this sinking to handle memory instructions.
751+
752+
// We optimistically assume we can sink all users after Previous. Keep a set
753+
// of instructions to sink after Previous ordered by dominance in the common
754+
// basic block. It will be applied to SinkAfter if all users can be sunk.
755+
auto CompareByComesBefore = [](const Instruction *A, const Instruction *B) {
756+
return A->comesBefore(B);
755757
};
758+
std::set<Instruction *, decltype(CompareByComesBefore)> InstrsToSink(
759+
CompareByComesBefore);
760+
761+
BasicBlock *PhiBB = Phi->getParent();
762+
SmallVector<Instruction *, 8> WorkList;
763+
auto TryToPushSinkCandidate = [&](Instruction *SinkCandidate) {
764+
// Already sunk SinkCandidate.
765+
if (SinkCandidate->getParent() == PhiBB &&
766+
InstrsToSink.find(SinkCandidate) != InstrsToSink.end())
767+
return true;
756768

757-
if (Phi->hasOneUse()) {
758-
Instruction *I = Phi->user_back();
759-
760-
// If the user of the PHI is also the incoming value, we potentially have a
761-
// reduction and which cannot be handled by sinking.
762-
if (Previous == I)
769+
// Cyclic dependence.
770+
if (Previous == SinkCandidate)
763771
return false;
764772

765-
// We cannot sink terminator instructions.
766-
if (I->getParent()->getTerminator() == I)
773+
if (DT->dominates(Previous,
774+
SinkCandidate)) // We already are good w/o sinking.
775+
return true;
776+
777+
if (SinkCandidate->getParent() != PhiBB ||
778+
SinkCandidate->mayHaveSideEffects() ||
779+
SinkCandidate->mayReadFromMemory() || SinkCandidate->isTerminator())
767780
return false;
768781

769782
// Do not try to sink an instruction multiple times (if multiple operands
770783
// are first order recurrences).
771784
// TODO: We can support this case, by sinking the instruction after the
772785
// 'deepest' previous instruction.
773-
if (SinkAfter.find(I) != SinkAfter.end())
786+
if (SinkAfter.find(SinkCandidate) != SinkAfter.end())
774787
return false;
775788

776-
if (DT->dominates(Previous, I)) // We already are good w/o sinking.
789+
// If we reach a PHI node that is not dominated by Previous, we reached a
790+
// header PHI. No need for sinking.
791+
if (isa<PHINode>(SinkCandidate))
777792
return true;
778793

779-
// We can sink any instruction without side effects, as long as all users
780-
// are dominated by the instruction we are sinking after.
781-
if (I->getParent() == Phi->getParent() && !I->mayHaveSideEffects() &&
782-
allUsesDominatedBy(I, Previous)) {
783-
SinkAfter[I] = Previous;
784-
return true;
794+
// Sink User tentatively and check its users
795+
InstrsToSink.insert(SinkCandidate);
796+
WorkList.push_back(SinkCandidate);
797+
return true;
798+
};
799+
800+
WorkList.push_back(Phi);
801+
// Try to recursively sink instructions and their users after Previous.
802+
while (!WorkList.empty()) {
803+
Instruction *Current = WorkList.pop_back_val();
804+
for (User *User : Current->users()) {
805+
if (!TryToPushSinkCandidate(cast<Instruction>(User)))
806+
return false;
785807
}
786808
}
787809

788-
return allUsesDominatedBy(Phi, Previous);
810+
// We can sink all users of Phi. Update the mapping.
811+
for (Instruction *I : InstrsToSink) {
812+
SinkAfter[I] = Previous;
813+
Previous = I;
814+
}
815+
return true;
789816
}
790817

791818
/// This function returns the identity element (or neutral element) for

llvm/lib/Transforms/Vectorize/LoopVectorizationPlanner.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,7 @@ class LoopVectorizationPlanner {
344344
/// Legal. This method is only used for the legacy inner loop vectorizer.
345345
VPlanPtr buildVPlanWithVPRecipes(
346346
VFRange &Range, SmallPtrSetImpl<Instruction *> &DeadInstructions,
347-
const DenseMap<Instruction *, Instruction *> &SinkAfter);
347+
const MapVector<Instruction *, Instruction *> &SinkAfter);
348348

349349
/// Build VPlans for power-of-2 VF's between \p MinVF and \p MaxVF inclusive,
350350
/// according to the information gathered by Legal when it checked if it is

llvm/lib/Transforms/Vectorize/LoopVectorize.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8994,7 +8994,7 @@ void LoopVectorizationPlanner::buildVPlansWithVPRecipes(ElementCount MinVF,
89948994
auto &ConditionalAssumes = Legal->getConditionalAssumes();
89958995
DeadInstructions.insert(ConditionalAssumes.begin(), ConditionalAssumes.end());
89968996

8997-
DenseMap<Instruction *, Instruction *> &SinkAfter = Legal->getSinkAfter();
8997+
MapVector<Instruction *, Instruction *> &SinkAfter = Legal->getSinkAfter();
89988998
// Dead instructions do not need sinking. Remove them from SinkAfter.
89998999
for (Instruction *I : DeadInstructions)
90009000
SinkAfter.erase(I);
@@ -9010,7 +9010,7 @@ void LoopVectorizationPlanner::buildVPlansWithVPRecipes(ElementCount MinVF,
90109010

90119011
VPlanPtr LoopVectorizationPlanner::buildVPlanWithVPRecipes(
90129012
VFRange &Range, SmallPtrSetImpl<Instruction *> &DeadInstructions,
9013-
const DenseMap<Instruction *, Instruction *> &SinkAfter) {
9013+
const MapVector<Instruction *, Instruction *> &SinkAfter) {
90149014

90159015
SmallPtrSet<const InterleaveGroup<Instruction> *, 1> InterleaveGroups;
90169016

0 commit comments

Comments
 (0)