[llvm] f4f673e - [NFC][InstCombine] PHI-aware aggregate reconstruction: don't capture UseBB in lambdas, take it as argument
Roman Lebedev via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 17 14:45:43 PDT 2020
Author: Roman Lebedev
Date: 2020-08-18T00:45:18+03:00
New Revision: f4f673e0e36937954c2410b2dfd5ca8e39ccffa5
URL: https://github.com/llvm/llvm-project/commit/f4f673e0e36937954c2410b2dfd5ca8e39ccffa5
DIFF: https://github.com/llvm/llvm-project/commit/f4f673e0e36937954c2410b2dfd5ca8e39ccffa5.diff
LOG: [NFC][InstCombine] PHI-aware aggregate reconstruction: don't capture UseBB in lambdas, take it as argument
In a following patch, UseBB will be detected later,
so capturing it is potentially error-prone (capture by ref vs by val).
Also, parametrized UseBB will likely be needed
for multiple levels of PHI indirections later on anyways.
Added:
Modified:
llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp b/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
index 6f1f96f6a203..f589a7fc03bc 100644
--- a/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
+++ b/llvm/lib/Transforms/InstCombine/InstCombineVectorOps.cpp
@@ -706,7 +706,6 @@ static ShuffleOps collectShuffleElements(Value *V, SmallVectorImpl<int> &Mask,
/// This potentially deals with PHI indirections.
Instruction *InstCombinerImpl::foldAggregateConstructionIntoAggregateReuse(
InsertValueInst &OrigIVI) {
- BasicBlock *UseBB = OrigIVI.getParent();
Type *AggTy = OrigIVI.getType();
unsigned NumAggElts;
switch (AggTy->getTypeID()) {
@@ -806,11 +805,11 @@ Instruction *InstCombinerImpl::foldAggregateConstructionIntoAggregateReuse(
// If found, return the source aggregate from which the extraction was.
// If \p PredBB is provided, does PHI translation of an \p Elt first.
auto FindSourceAggregate =
- [&](Value *Elt, unsigned EltIdx,
+ [&](Value *Elt, unsigned EltIdx, Optional<BasicBlock *> UseBB,
Optional<BasicBlock *> PredBB) -> Optional<Value *> {
// For now(?), only deal with, at most, a single level of PHI indirection.
- if (PredBB)
- Elt = Elt->DoPHITranslation(UseBB, *PredBB);
+ if (UseBB && PredBB)
+ Elt = Elt->DoPHITranslation(*UseBB, *PredBB);
// FIXME: deal with multiple levels of PHI indirection?
// Did we find an extraction?
@@ -834,7 +833,8 @@ Instruction *InstCombinerImpl::foldAggregateConstructionIntoAggregateReuse(
// see if we can find appropriate source aggregate for each of the elements,
// and see it's the same aggregate for each element. If so, return it.
auto FindCommonSourceAggregate =
- [&](Optional<BasicBlock *> PredBB) -> Optional<Value *> {
+ [&](Optional<BasicBlock *> UseBB,
+ Optional<BasicBlock *> PredBB) -> Optional<Value *> {
Optional<Value *> SourceAggregate;
for (auto I : enumerate(AggElts)) {
@@ -848,7 +848,7 @@ Instruction *InstCombinerImpl::foldAggregateConstructionIntoAggregateReuse(
// FIXME: we could special-case undef element, IFF we know that in the
// source aggregate said element isn't poison.
Optional<Value *> SourceAggregateForElement =
- FindSourceAggregate(*I.value(), I.index(), PredBB);
+ FindSourceAggregate(*I.value(), I.index(), UseBB, PredBB);
// Okay, what have we found? Does that correlate with previous findings?
@@ -884,7 +884,7 @@ Instruction *InstCombinerImpl::foldAggregateConstructionIntoAggregateReuse(
Optional<Value *> SourceAggregate;
// Can we find the source aggregate without looking at predecessors?
- SourceAggregate = FindCommonSourceAggregate(/*PredBB=*/None);
+ SourceAggregate = FindCommonSourceAggregate(/*UseBB=*/None, /*PredBB=*/None);
if (Describe(SourceAggregate) != AggregateDescription::NotFound) {
if (Describe(SourceAggregate) == AggregateDescription::FoundMismatch)
return nullptr; // Conflicting source aggregates!
@@ -892,13 +892,21 @@ Instruction *InstCombinerImpl::foldAggregateConstructionIntoAggregateReuse(
return replaceInstUsesWith(OrigIVI, *SourceAggregate);
}
+ // Okay, apparently we need to look at predecessors.
+
+ BasicBlock *UseBB = OrigIVI.getParent();
+
+ // If *all* of the elements are basic-block-independent, meaning they are
+ // either function arguments, or constant expressions, then if we didn't
+ // handle them without predecessor-aware folding, we won't handle them now.
+ if (!UseBB)
+ return nullptr;
+
// If we didn't manage to find source aggregate without looking at
// predecessors, and there are no predecessors to look at, then we're done.
if (pred_empty(UseBB))
return nullptr;
- // Okay, apparently we need to look at predecessors.
-
// Arbitrary predecessor count limit.
static const int PredCountLimit = 64;
// Don't bother if there are too many predecessors.
@@ -909,9 +917,9 @@ Instruction *InstCombinerImpl::foldAggregateConstructionIntoAggregateReuse(
// from which all the elements were originally extracted from?
// Note that we want for the map to have stable iteration order!
SmallMapVector<BasicBlock *, Value *, 4> SourceAggregates;
- for (BasicBlock *Pred : predecessors(UseBB)) {
+ for (BasicBlock *PredBB : predecessors(UseBB)) {
std::pair<decltype(SourceAggregates)::iterator, bool> IV =
- SourceAggregates.insert({Pred, nullptr});
+ SourceAggregates.insert({PredBB, nullptr});
// Did we already evaluate this predecessor?
if (!IV.second)
continue;
@@ -919,7 +927,7 @@ Instruction *InstCombinerImpl::foldAggregateConstructionIntoAggregateReuse(
// Let's hope that when coming from predecessor Pred, all elements of the
// aggregate produced by OrigIVI must have been originally extracted from
// the same aggregate. Is that so? Can we find said original aggregate?
- SourceAggregate = FindCommonSourceAggregate(Pred);
+ SourceAggregate = FindCommonSourceAggregate(UseBB, PredBB);
if (Describe(SourceAggregate) != AggregateDescription::Found)
return nullptr; // Give up.
IV.first->second = *SourceAggregate;
More information about the llvm-commits
mailing list