@@ -963,6 +963,9 @@ Instruction *InstCombinerImpl::foldAggregateConstructionIntoAggregateReuse(
963
963
return AggregateDescription::Found;
964
964
};
965
965
966
+ // If an aggregate element is defined in UseBB, we can't use it in PredBB.
967
+ bool EltDefinedInUseBB = false ;
968
+
966
969
// Given the value \p Elt that was being inserted into element \p EltIdx of an
967
970
// aggregate AggTy, see if \p Elt was originally defined by an
968
971
// appropriate extractvalue (same element index, same aggregate type).
@@ -972,8 +975,11 @@ Instruction *InstCombinerImpl::foldAggregateConstructionIntoAggregateReuse(
972
975
[&](Instruction *Elt, unsigned EltIdx, std::optional<BasicBlock *> UseBB,
973
976
std::optional<BasicBlock *> PredBB) -> std::optional<Value *> {
974
977
// For now(?), only deal with, at most, a single level of PHI indirection.
975
- if (UseBB && PredBB)
978
+ if (UseBB && PredBB) {
976
979
Elt = dyn_cast<Instruction>(Elt->DoPHITranslation (*UseBB, *PredBB));
980
+ if (Elt && Elt->getParent () == *UseBB)
981
+ EltDefinedInUseBB = true ;
982
+ }
977
983
// FIXME: deal with multiple levels of PHI indirection?
978
984
979
985
// Did we find an extraction?
@@ -1106,6 +1112,7 @@ Instruction *InstCombinerImpl::foldAggregateConstructionIntoAggregateReuse(
1106
1112
// from which all the elements were originally extracted from?
1107
1113
// Note that we want for the map to have stable iteration order!
1108
1114
SmallDenseMap<BasicBlock *, Value *, 4 > SourceAggregates;
1115
+ bool FoundSrcAgg = false ;
1109
1116
for (BasicBlock *Pred : Preds) {
1110
1117
std::pair<decltype (SourceAggregates)::iterator, bool > IV =
1111
1118
SourceAggregates.insert ({Pred, nullptr });
@@ -1117,9 +1124,68 @@ Instruction *InstCombinerImpl::foldAggregateConstructionIntoAggregateReuse(
1117
1124
// aggregate produced by OrigIVI must have been originally extracted from
1118
1125
// the same aggregate. Is that so? Can we find said original aggregate?
1119
1126
SourceAggregate = FindCommonSourceAggregate (UseBB, Pred);
1120
- if (Describe (SourceAggregate) != AggregateDescription::Found)
1121
- return nullptr ; // Give up.
1122
- IV.first ->second = *SourceAggregate;
1127
+ if (Describe (SourceAggregate) == AggregateDescription::Found) {
1128
+ FoundSrcAgg = true ;
1129
+ IV.first ->second = *SourceAggregate;
1130
+ } else {
1131
+ // If UseBB is the single successor of Pred, we can add InsertValue to
1132
+ // Pred.
1133
+ auto *BI = dyn_cast<BranchInst>(Pred->getTerminator ());
1134
+ if (!BI || !BI->isUnconditional ())
1135
+ return nullptr ;
1136
+ }
1137
+ }
1138
+
1139
+ if (!FoundSrcAgg)
1140
+ return nullptr ;
1141
+
1142
+ // Do some sanity check if we need to add insertvalue into predecessors.
1143
+ auto OrigBB = OrigIVI.getParent ();
1144
+ for (auto &It : SourceAggregates) {
1145
+ if (Describe (It.second ) == AggregateDescription::Found)
1146
+ continue ;
1147
+
1148
+ // Element is defined in UseBB, so it can't be used in predecessors.
1149
+ if (EltDefinedInUseBB)
1150
+ return nullptr ;
1151
+
1152
+ // Do this transformation cross loop boundary may cause dead loop. So we
1153
+ // should avoid this situation. But LoopInfo is not generally available, we
1154
+ // must be conservative here.
1155
+ // If OrigIVI is in UseBB and it's the only successor of PredBB, PredBB
1156
+ // can't be in inner loop.
1157
+ if (UseBB != OrigBB)
1158
+ return nullptr ;
1159
+
1160
+ // Avoid constructing constant aggregate because constant value may expose
1161
+ // more optimizations.
1162
+ bool ConstAgg = true ;
1163
+ for (auto Val : AggElts) {
1164
+ Value *Elt = (*Val)->DoPHITranslation (UseBB, It.first );
1165
+ if (!isa<Constant>(Elt)) {
1166
+ ConstAgg = false ;
1167
+ break ;
1168
+ }
1169
+ }
1170
+ if (ConstAgg)
1171
+ return nullptr ;
1172
+ }
1173
+
1174
+ // For predecessors without appropriate source aggregate, create one in the
1175
+ // predecessor.
1176
+ for (auto &It : SourceAggregates) {
1177
+ if (Describe (It.second ) == AggregateDescription::Found)
1178
+ continue ;
1179
+
1180
+ BasicBlock *Pred = It.first ;
1181
+ Builder.SetInsertPoint (Pred->getTerminator ());
1182
+ Value *V = PoisonValue::get (AggTy);
1183
+ for (auto [Idx, Val] : enumerate(AggElts)) {
1184
+ Value *Elt = (*Val)->DoPHITranslation (UseBB, Pred);
1185
+ V = Builder.CreateInsertValue (V, Elt, Idx);
1186
+ }
1187
+
1188
+ It.second = V;
1123
1189
}
1124
1190
1125
1191
// All good! Now we just need to thread the source aggregates here.
0 commit comments