Skip to content

Commit 4f02a0f

Browse files
[NFC][CodeGenPrepare] Match against the correct instruction when checking profitability of folding an address
The "nested" `AddressingModeMatcher`s in `AddressingModeMatcher::isProfitableToFoldIntoAddressingMode` are constructed using the original memory instruction, even though they check whether the address operand of a differrent memory instructon is foldable. The memory instruction is used only for a dominance check (when not checking for profitability), and using the wrong memory instruction does not change the outcome of the test - if an address is foldable, the dominance test afects which of the two possible ways to fold is chosen, but this result is discarded. As an example, in target triple = "x86_64-linux" declare i1 @check(i64, i64) define i32 @f(i1 %cc, ptr %p, ptr %q, i64 %n) { entry: br label %loop loop: %iv = phi i64 [ %i, %C ], [ 0, %entry ] %offs = mul i64 %iv, 4 %c.0 = icmp ult i64 %iv, %n br i1 %c.0, label %A, label %fail A: br i1 %cc, label %B, label %C C: %u = phi i32 [0, %A], [%w, %B] %i = add i64 %iv, 1 %a.0 = getelementptr i8, ptr %p, i64 %offs %a.1 = getelementptr i8, ptr %a.0, i64 4 %v = load i32, ptr %a.1 %c.1 = icmp eq i32 %v, %u br i1 %c.1, label %exit, label %loop B: %a.2 = getelementptr i8, ptr %p, i64 %offs %a.3 = getelementptr i8, ptr %a.2, i64 4 %w = load i32, ptr %a.3 br label %C exit: ret i32 -1 fail: ret i32 0 } the dominance test is perfomed between `%i = ...` and `%v = ...` at the moment we're checking whether `%a3 = ...` is foldable Using the memory instruction, which uses the interesting address is "more correct" and this change is needed by a future patch. Reviewed By: mkazantsev Differential Revision: https://reviews.llvm.org/D143896
1 parent f84ac48 commit 4f02a0f

File tree

1 file changed

+11
-10
lines changed

1 file changed

+11
-10
lines changed

llvm/lib/CodeGen/CodeGenPrepare.cpp

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4976,7 +4976,7 @@ static bool IsOperandAMemoryOperand(CallInst *CI, InlineAsm *IA, Value *OpVal,
49764976
/// If we find an obviously non-foldable instruction, return true.
49774977
/// Add accessed addresses and types to MemoryUses.
49784978
static bool FindAllMemoryUses(
4979-
Instruction *I, SmallVectorImpl<std::pair<Value *, Type *>> &MemoryUses,
4979+
Instruction *I, SmallVectorImpl<std::pair<Use *, Type *>> &MemoryUses,
49804980
SmallPtrSetImpl<Instruction *> &ConsideredInsts, const TargetLowering &TLI,
49814981
const TargetRegisterInfo &TRI, bool OptSize, ProfileSummaryInfo *PSI,
49824982
BlockFrequencyInfo *BFI, unsigned &SeenInsts) {
@@ -4997,28 +4997,28 @@ static bool FindAllMemoryUses(
49974997

49984998
Instruction *UserI = cast<Instruction>(U.getUser());
49994999
if (LoadInst *LI = dyn_cast<LoadInst>(UserI)) {
5000-
MemoryUses.push_back({U.get(), LI->getType()});
5000+
MemoryUses.push_back({&U, LI->getType()});
50015001
continue;
50025002
}
50035003

50045004
if (StoreInst *SI = dyn_cast<StoreInst>(UserI)) {
50055005
if (U.getOperandNo() != StoreInst::getPointerOperandIndex())
50065006
return true; // Storing addr, not into addr.
5007-
MemoryUses.push_back({U.get(), SI->getValueOperand()->getType()});
5007+
MemoryUses.push_back({&U, SI->getValueOperand()->getType()});
50085008
continue;
50095009
}
50105010

50115011
if (AtomicRMWInst *RMW = dyn_cast<AtomicRMWInst>(UserI)) {
50125012
if (U.getOperandNo() != AtomicRMWInst::getPointerOperandIndex())
50135013
return true; // Storing addr, not into addr.
5014-
MemoryUses.push_back({U.get(), RMW->getValOperand()->getType()});
5014+
MemoryUses.push_back({&U, RMW->getValOperand()->getType()});
50155015
continue;
50165016
}
50175017

50185018
if (AtomicCmpXchgInst *CmpX = dyn_cast<AtomicCmpXchgInst>(UserI)) {
50195019
if (U.getOperandNo() != AtomicCmpXchgInst::getPointerOperandIndex())
50205020
return true; // Storing addr, not into addr.
5021-
MemoryUses.push_back({U.get(), CmpX->getCompareOperand()->getType()});
5021+
MemoryUses.push_back({&U, CmpX->getCompareOperand()->getType()});
50225022
continue;
50235023
}
50245024

@@ -5051,7 +5051,7 @@ static bool FindAllMemoryUses(
50515051
}
50525052

50535053
static bool FindAllMemoryUses(
5054-
Instruction *I, SmallVectorImpl<std::pair<Value *, Type *>> &MemoryUses,
5054+
Instruction *I, SmallVectorImpl<std::pair<Use *, Type *>> &MemoryUses,
50555055
const TargetLowering &TLI, const TargetRegisterInfo &TRI, bool OptSize,
50565056
ProfileSummaryInfo *PSI, BlockFrequencyInfo *BFI) {
50575057
unsigned SeenInsts = 0;
@@ -5142,7 +5142,7 @@ bool AddressingModeMatcher::isProfitableToFoldIntoAddressingMode(
51425142
// we can remove the addressing mode and effectively trade one live register
51435143
// for another (at worst.) In this context, folding an addressing mode into
51445144
// the use is just a particularly nice way of sinking it.
5145-
SmallVector<std::pair<Value *, Type *>, 16> MemoryUses;
5145+
SmallVector<std::pair<Use *, Type *>, 16> MemoryUses;
51465146
if (FindAllMemoryUses(I, MemoryUses, TLI, TRI, OptSize, PSI, BFI))
51475147
return false; // Has a non-memory, non-foldable use!
51485148

@@ -5156,8 +5156,9 @@ bool AddressingModeMatcher::isProfitableToFoldIntoAddressingMode(
51565156
// growth since most architectures have some reasonable small and fast way to
51575157
// compute an effective address. (i.e LEA on x86)
51585158
SmallVector<Instruction *, 32> MatchedAddrModeInsts;
5159-
for (const std::pair<Value *, Type *> &Pair : MemoryUses) {
5160-
Value *Address = Pair.first;
5159+
for (const std::pair<Use *, Type *> &Pair : MemoryUses) {
5160+
Value *Address = Pair.first->get();
5161+
Instruction *UserI = cast<Instruction>(Pair.first->getUser());
51615162
Type *AddressAccessTy = Pair.second;
51625163
unsigned AS = Address->getType()->getPointerAddressSpace();
51635164

@@ -5170,7 +5171,7 @@ bool AddressingModeMatcher::isProfitableToFoldIntoAddressingMode(
51705171
TypePromotionTransaction::ConstRestorationPt LastKnownGood =
51715172
TPT.getRestorationPoint();
51725173
AddressingModeMatcher Matcher(MatchedAddrModeInsts, TLI, TRI, LI, getDTFn,
5173-
AddressAccessTy, AS, MemoryInst, Result,
5174+
AddressAccessTy, AS, UserI, Result,
51745175
InsertedInsts, PromotedInsts, TPT,
51755176
LargeOffsetGEP, OptSize, PSI, BFI);
51765177
Matcher.IgnoreProfitability = true;

0 commit comments

Comments
 (0)