Skip to content

Commit 465f793

Browse files
authored
[SeparateConstOffsetFromGEP] Highlight that trunc is handled. NFC (#154563)
Update code comments and variable/function names to make it more clear that we handle trunc instructions (and not only sext/zext) when extracting constant offsets from a GEP index expressions. This for example renames the vector ExtInsts to CastInsts.
1 parent faa7a87 commit 465f793

File tree

1 file changed

+39
-37
lines changed

1 file changed

+39
-37
lines changed

llvm/lib/Transforms/Scalar/SeparateConstOffsetFromGEP.cpp

Lines changed: 39 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -247,8 +247,8 @@ class ConstantOffsetExtractor {
247247
/// index I' according to UserChain produced by function "find".
248248
///
249249
/// The building conceptually takes two steps:
250-
/// 1) iteratively distribute s/zext towards the leaves of the expression tree
251-
/// that computes I
250+
/// 1) iteratively distribute sext/zext/trunc towards the leaves of the
251+
/// expression tree that computes I
252252
/// 2) reassociate the expression tree to the form I' + C.
253253
///
254254
/// For example, to extract the 5 from sext(a + (b + 5)), we first distribute
@@ -260,29 +260,30 @@ class ConstantOffsetExtractor {
260260
Value *rebuildWithoutConstOffset();
261261

262262
/// After the first step of rebuilding the GEP index without the constant
263-
/// offset, distribute s/zext to the operands of all operators in UserChain.
264-
/// e.g., zext(sext(a + (b + 5)) (assuming no overflow) =>
263+
/// offset, distribute sext/zext/trunc to the operands of all operators in
264+
/// UserChain. e.g., zext(sext(a + (b + 5)) (assuming no overflow) =>
265265
/// zext(sext(a)) + (zext(sext(b)) + zext(sext(5))).
266266
///
267267
/// The function also updates UserChain to point to new subexpressions after
268-
/// distributing s/zext. e.g., the old UserChain of the above example is
269-
/// 5 -> b + 5 -> a + (b + 5) -> sext(...) -> zext(sext(...)),
268+
/// distributing sext/zext/trunc. e.g., the old UserChain of the above example
269+
/// is
270+
/// 5 -> b + 5 -> a + (b + 5) -> sext(...) -> zext(sext(...)),
270271
/// and the new UserChain is
271-
/// zext(sext(5)) -> zext(sext(b)) + zext(sext(5)) ->
272-
/// zext(sext(a)) + (zext(sext(b)) + zext(sext(5))
272+
/// zext(sext(5)) -> zext(sext(b)) + zext(sext(5)) ->
273+
/// zext(sext(a)) + (zext(sext(b)) + zext(sext(5))
273274
///
274275
/// \p ChainIndex The index to UserChain. ChainIndex is initially
275276
/// UserChain.size() - 1, and is decremented during
276277
/// the recursion.
277-
Value *distributeExtsAndCloneChain(unsigned ChainIndex);
278+
Value *distributeCastsAndCloneChain(unsigned ChainIndex);
278279

279280
/// Reassociates the GEP index to the form I' + C and returns I'.
280281
Value *removeConstOffset(unsigned ChainIndex);
281282

282-
/// A helper function to apply ExtInsts, a list of s/zext, to value V.
283-
/// e.g., if ExtInsts = [sext i32 to i64, zext i16 to i32], this function
283+
/// A helper function to apply CastInsts, a list of sext/zext/trunc, to value
284+
/// V. e.g., if CastInsts = [sext i32 to i64, zext i16 to i32], this function
284285
/// returns "sext i32 (zext i16 V to i32) to i64".
285-
Value *applyExts(Value *V);
286+
Value *applyCasts(Value *V);
286287

287288
/// A helper function that returns whether we can trace into the operands
288289
/// of binary operator BO for a constant offset.
@@ -307,8 +308,8 @@ class ConstantOffsetExtractor {
307308
SmallVector<User *, 8> UserChain;
308309

309310
/// A data structure used in rebuildWithoutConstOffset. Contains all
310-
/// sext/zext instructions along UserChain.
311-
SmallVector<CastInst *, 16> ExtInsts;
311+
/// sext/zext/trunc instructions along UserChain.
312+
SmallVector<CastInst *, 16> CastInsts;
312313

313314
/// Insertion position of cloned instructions.
314315
BasicBlock::iterator IP;
@@ -491,7 +492,7 @@ bool ConstantOffsetExtractor::CanTraceInto(bool SignExtended,
491492
}
492493

493494
Value *LHS = BO->getOperand(0), *RHS = BO->getOperand(1);
494-
// Do not trace into "or" unless it is equivalent to "add".
495+
// Do not trace into "or" unless it is equivalent to "add nuw nsw".
495496
// This is the case if the or's disjoint flag is set.
496497
if (BO->getOpcode() == Instruction::Or &&
497498
!cast<PossiblyDisjointInst>(BO)->isDisjoint())
@@ -503,8 +504,8 @@ bool ConstantOffsetExtractor::CanTraceInto(bool SignExtended,
503504
if (ZeroExtended && !SignExtended && BO->getOpcode() == Instruction::Sub)
504505
return false;
505506

506-
// In addition, tracing into BO requires that its surrounding s/zext (if
507-
// any) is distributable to both operands.
507+
// In addition, tracing into BO requires that its surrounding sext/zext/trunc
508+
// (if any) is distributable to both operands.
508509
//
509510
// Suppose BO = A op B.
510511
// SignExtended | ZeroExtended | Distributable?
@@ -628,36 +629,36 @@ APInt ConstantOffsetExtractor::find(Value *V, bool SignExtended,
628629
return ConstantOffset;
629630
}
630631

631-
Value *ConstantOffsetExtractor::applyExts(Value *V) {
632+
Value *ConstantOffsetExtractor::applyCasts(Value *V) {
632633
Value *Current = V;
633-
// ExtInsts is built in the use-def order. Therefore, we apply them to V
634+
// CastInsts is built in the use-def order. Therefore, we apply them to V
634635
// in the reversed order.
635-
for (CastInst *I : llvm::reverse(ExtInsts)) {
636+
for (CastInst *I : llvm::reverse(CastInsts)) {
636637
if (Constant *C = dyn_cast<Constant>(Current)) {
637638
// Try to constant fold the cast.
638639
Current = ConstantFoldCastOperand(I->getOpcode(), C, I->getType(), DL);
639640
if (Current)
640641
continue;
641642
}
642643

643-
Instruction *Ext = I->clone();
644-
Ext->setOperand(0, Current);
644+
Instruction *Cast = I->clone();
645+
Cast->setOperand(0, Current);
645646
// In ConstantOffsetExtractor::find we do not analyze nuw/nsw for trunc, so
646647
// we assume that it is ok to redistribute trunc over add/sub/or. But for
647648
// example (add (trunc nuw A), (trunc nuw B)) is more poisonous than (trunc
648649
// nuw (add A, B))). To make such redistributions legal we drop all the
649650
// poison generating flags from cloned trunc instructions here.
650-
if (isa<TruncInst>(Ext))
651-
Ext->dropPoisonGeneratingFlags();
652-
Ext->insertBefore(*IP->getParent(), IP);
653-
Current = Ext;
651+
if (isa<TruncInst>(Cast))
652+
Cast->dropPoisonGeneratingFlags();
653+
Cast->insertBefore(*IP->getParent(), IP);
654+
Current = Cast;
654655
}
655656
return Current;
656657
}
657658

658659
Value *ConstantOffsetExtractor::rebuildWithoutConstOffset() {
659-
distributeExtsAndCloneChain(UserChain.size() - 1);
660-
// Remove all nullptrs (used to be s/zext) from UserChain.
660+
distributeCastsAndCloneChain(UserChain.size() - 1);
661+
// Remove all nullptrs (used to be sext/zext/trunc) from UserChain.
661662
unsigned NewSize = 0;
662663
for (User *I : UserChain) {
663664
if (I != nullptr) {
@@ -670,29 +671,29 @@ Value *ConstantOffsetExtractor::rebuildWithoutConstOffset() {
670671
}
671672

672673
Value *
673-
ConstantOffsetExtractor::distributeExtsAndCloneChain(unsigned ChainIndex) {
674+
ConstantOffsetExtractor::distributeCastsAndCloneChain(unsigned ChainIndex) {
674675
User *U = UserChain[ChainIndex];
675676
if (ChainIndex == 0) {
676677
assert(isa<ConstantInt>(U));
677-
// If U is a ConstantInt, applyExts will return a ConstantInt as well.
678-
return UserChain[ChainIndex] = cast<ConstantInt>(applyExts(U));
678+
// If U is a ConstantInt, applyCasts will return a ConstantInt as well.
679+
return UserChain[ChainIndex] = cast<ConstantInt>(applyCasts(U));
679680
}
680681

681682
if (CastInst *Cast = dyn_cast<CastInst>(U)) {
682683
assert(
683684
(isa<SExtInst>(Cast) || isa<ZExtInst>(Cast) || isa<TruncInst>(Cast)) &&
684685
"Only following instructions can be traced: sext, zext & trunc");
685-
ExtInsts.push_back(Cast);
686+
CastInsts.push_back(Cast);
686687
UserChain[ChainIndex] = nullptr;
687-
return distributeExtsAndCloneChain(ChainIndex - 1);
688+
return distributeCastsAndCloneChain(ChainIndex - 1);
688689
}
689690

690691
// Function find only trace into BinaryOperator and CastInst.
691692
BinaryOperator *BO = cast<BinaryOperator>(U);
692693
// OpNo = which operand of BO is UserChain[ChainIndex - 1]
693694
unsigned OpNo = (BO->getOperand(0) == UserChain[ChainIndex - 1] ? 0 : 1);
694-
Value *TheOther = applyExts(BO->getOperand(1 - OpNo));
695-
Value *NextInChain = distributeExtsAndCloneChain(ChainIndex - 1);
695+
Value *TheOther = applyCasts(BO->getOperand(1 - OpNo));
696+
Value *NextInChain = distributeCastsAndCloneChain(ChainIndex - 1);
696697

697698
BinaryOperator *NewBO = nullptr;
698699
if (OpNo == 0) {
@@ -713,7 +714,7 @@ Value *ConstantOffsetExtractor::removeConstOffset(unsigned ChainIndex) {
713714

714715
BinaryOperator *BO = cast<BinaryOperator>(UserChain[ChainIndex]);
715716
assert((BO->use_empty() || BO->hasOneUse()) &&
716-
"distributeExtsAndCloneChain clones each BinaryOperator in "
717+
"distributeCastsAndCloneChain clones each BinaryOperator in "
717718
"UserChain, so no one should be used more than "
718719
"once");
719720

@@ -847,7 +848,8 @@ static bool allowsPreservingNUW(const User *U) {
847848
// "add nuw trunc(a), trunc(b)" is more poisonous than "trunc(add nuw a, b)"
848849
if (const TruncInst *TI = dyn_cast<TruncInst>(U))
849850
return TI->hasNoUnsignedWrap();
850-
return isa<CastInst>(U) || isa<ConstantInt>(U);
851+
assert((isa<CastInst>(U) || isa<ConstantInt>(U)) && "Unexpected User.");
852+
return true;
851853
}
852854

853855
Value *ConstantOffsetExtractor::Extract(Value *Idx, GetElementPtrInst *GEP,

0 commit comments

Comments
 (0)