Skip to content

Commit 2c768c7

Browse files
author
Krzysztof Parzyszek
committed
[EarlyCSE] Small refactoring changes, NFC
1. Store intrinsic ID in ParseMemoryInst instead of a boolean flag "IsTargetMemInst". This will make it easier to add support for target-independent intrinsics. 2. Extract the complex multiline conditions from EarlyCSE::processNode into a new function "getMatchingValue". Differential Revision: https://reviews.llvm.org/D87691
1 parent bb82135 commit 2c768c7

File tree

1 file changed

+66
-43
lines changed

1 file changed

+66
-43
lines changed

llvm/lib/Transforms/Scalar/EarlyCSE.cpp

Lines changed: 66 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -688,29 +688,35 @@ class EarlyCSE {
688688
public:
689689
ParseMemoryInst(Instruction *Inst, const TargetTransformInfo &TTI)
690690
: Inst(Inst) {
691-
if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(Inst))
691+
if (IntrinsicInst *II = dyn_cast<IntrinsicInst>(Inst)) {
692692
if (TTI.getTgtMemIntrinsic(II, Info))
693-
IsTargetMemInst = true;
693+
IntrID = II->getIntrinsicID();
694+
}
694695
}
695696

697+
Instruction *get() { return Inst; }
698+
const Instruction *get() const { return Inst; }
699+
696700
bool isLoad() const {
697-
if (IsTargetMemInst) return Info.ReadMem;
701+
if (IntrID != 0)
702+
return Info.ReadMem;
698703
return isa<LoadInst>(Inst);
699704
}
700705

701706
bool isStore() const {
702-
if (IsTargetMemInst) return Info.WriteMem;
707+
if (IntrID != 0)
708+
return Info.WriteMem;
703709
return isa<StoreInst>(Inst);
704710
}
705711

706712
bool isAtomic() const {
707-
if (IsTargetMemInst)
713+
if (IntrID != 0)
708714
return Info.Ordering != AtomicOrdering::NotAtomic;
709715
return Inst->isAtomic();
710716
}
711717

712718
bool isUnordered() const {
713-
if (IsTargetMemInst)
719+
if (IntrID != 0)
714720
return Info.isUnordered();
715721

716722
if (LoadInst *LI = dyn_cast<LoadInst>(Inst)) {
@@ -723,7 +729,7 @@ class EarlyCSE {
723729
}
724730

725731
bool isVolatile() const {
726-
if (IsTargetMemInst)
732+
if (IntrID != 0)
727733
return Info.IsVolatile;
728734

729735
if (LoadInst *LI = dyn_cast<LoadInst>(Inst)) {
@@ -753,27 +759,31 @@ class EarlyCSE {
753759
// field in the MemIntrinsicInfo structure. That field contains
754760
// non-negative values only.
755761
int getMatchingId() const {
756-
if (IsTargetMemInst) return Info.MatchingId;
762+
if (IntrID != 0)
763+
return Info.MatchingId;
757764
return -1;
758765
}
759766

760767
Value *getPointerOperand() const {
761-
if (IsTargetMemInst) return Info.PtrVal;
768+
if (IntrID != 0)
769+
return Info.PtrVal;
762770
return getLoadStorePointerOperand(Inst);
763771
}
764772

765773
bool mayReadFromMemory() const {
766-
if (IsTargetMemInst) return Info.ReadMem;
774+
if (IntrID != 0)
775+
return Info.ReadMem;
767776
return Inst->mayReadFromMemory();
768777
}
769778

770779
bool mayWriteToMemory() const {
771-
if (IsTargetMemInst) return Info.WriteMem;
780+
if (IntrID != 0)
781+
return Info.WriteMem;
772782
return Inst->mayWriteToMemory();
773783
}
774784

775785
private:
776-
bool IsTargetMemInst = false;
786+
Intrinsic::ID IntrID = 0;
777787
MemIntrinsicInfo Info;
778788
Instruction *Inst;
779789
};
@@ -783,6 +793,9 @@ class EarlyCSE {
783793
bool handleBranchCondition(Instruction *CondInst, const BranchInst *BI,
784794
const BasicBlock *BB, const BasicBlock *Pred);
785795

796+
Value *getMatchingValue(LoadValue &InVal, ParseMemoryInst &MemInst,
797+
unsigned CurrentGeneration);
798+
786799
Value *getOrCreateResult(Value *Inst, Type *ExpectedType) const {
787800
if (auto *LI = dyn_cast<LoadInst>(Inst))
788801
return LI;
@@ -945,6 +958,33 @@ bool EarlyCSE::handleBranchCondition(Instruction *CondInst,
945958
return MadeChanges;
946959
}
947960

961+
Value *EarlyCSE::getMatchingValue(LoadValue &InVal, ParseMemoryInst &MemInst,
962+
unsigned CurrentGeneration) {
963+
if (InVal.DefInst == nullptr)
964+
return nullptr;
965+
if (InVal.MatchingId != MemInst.getMatchingId())
966+
return nullptr;
967+
// We don't yet handle removing loads with ordering of any kind.
968+
if (MemInst.isVolatile() || !MemInst.isUnordered())
969+
return nullptr;
970+
// We can't replace an atomic load with one which isn't also atomic.
971+
if (MemInst.isLoad() && !InVal.IsAtomic && MemInst.isAtomic())
972+
return nullptr;
973+
// The value V returned from this function is used differently depending
974+
// on whether MemInst is a load or a store. If it's a load, we will replace
975+
// MemInst with V, if it's a store, we will check if V is the same as the
976+
// available value.
977+
bool MemInstMatching = !MemInst.isLoad();
978+
Instruction *Matching = MemInstMatching ? MemInst.get() : InVal.DefInst;
979+
Instruction *Other = MemInstMatching ? InVal.DefInst : MemInst.get();
980+
981+
if (!isOperatingOnInvariantMemAt(MemInst.get(), InVal.Generation) &&
982+
!isSameMemGeneration(InVal.Generation, CurrentGeneration, InVal.DefInst,
983+
MemInst.get()))
984+
return nullptr;
985+
return getOrCreateResult(Matching, Other->getType());
986+
}
987+
948988
bool EarlyCSE::processNode(DomTreeNode *Node) {
949989
bool Changed = false;
950990
BasicBlock *BB = Node->getBlock();
@@ -1161,32 +1201,21 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
11611201
// we can assume the current load loads the same value as the dominating
11621202
// load.
11631203
LoadValue InVal = AvailableLoads.lookup(MemInst.getPointerOperand());
1164-
if (InVal.DefInst != nullptr &&
1165-
InVal.MatchingId == MemInst.getMatchingId() &&
1166-
// We don't yet handle removing loads with ordering of any kind.
1167-
!MemInst.isVolatile() && MemInst.isUnordered() &&
1168-
// We can't replace an atomic load with one which isn't also atomic.
1169-
InVal.IsAtomic >= MemInst.isAtomic() &&
1170-
(isOperatingOnInvariantMemAt(&Inst, InVal.Generation) ||
1171-
isSameMemGeneration(InVal.Generation, CurrentGeneration,
1172-
InVal.DefInst, &Inst))) {
1173-
Value *Op = getOrCreateResult(InVal.DefInst, Inst.getType());
1174-
if (Op != nullptr) {
1175-
LLVM_DEBUG(dbgs() << "EarlyCSE CSE LOAD: " << Inst
1176-
<< " to: " << *InVal.DefInst << '\n');
1177-
if (!DebugCounter::shouldExecute(CSECounter)) {
1178-
LLVM_DEBUG(dbgs() << "Skipping due to debug counter\n");
1179-
continue;
1180-
}
1181-
if (!Inst.use_empty())
1182-
Inst.replaceAllUsesWith(Op);
1183-
salvageKnowledge(&Inst, &AC);
1184-
removeMSSA(Inst);
1185-
Inst.eraseFromParent();
1186-
Changed = true;
1187-
++NumCSELoad;
1204+
if (Value *Op = getMatchingValue(InVal, MemInst, CurrentGeneration)) {
1205+
LLVM_DEBUG(dbgs() << "EarlyCSE CSE LOAD: " << Inst
1206+
<< " to: " << *InVal.DefInst << '\n');
1207+
if (!DebugCounter::shouldExecute(CSECounter)) {
1208+
LLVM_DEBUG(dbgs() << "Skipping due to debug counter\n");
11881209
continue;
11891210
}
1211+
if (!Inst.use_empty())
1212+
Inst.replaceAllUsesWith(Op);
1213+
salvageKnowledge(&Inst, &AC);
1214+
removeMSSA(Inst);
1215+
Inst.eraseFromParent();
1216+
Changed = true;
1217+
++NumCSELoad;
1218+
continue;
11901219
}
11911220

11921221
// Otherwise, remember that we have this instruction.
@@ -1256,13 +1285,7 @@ bool EarlyCSE::processNode(DomTreeNode *Node) {
12561285
if (MemInst.isValid() && MemInst.isStore()) {
12571286
LoadValue InVal = AvailableLoads.lookup(MemInst.getPointerOperand());
12581287
if (InVal.DefInst &&
1259-
InVal.DefInst == getOrCreateResult(&Inst, InVal.DefInst->getType()) &&
1260-
InVal.MatchingId == MemInst.getMatchingId() &&
1261-
// We don't yet handle removing stores with ordering of any kind.
1262-
!MemInst.isVolatile() && MemInst.isUnordered() &&
1263-
(isOperatingOnInvariantMemAt(&Inst, InVal.Generation) ||
1264-
isSameMemGeneration(InVal.Generation, CurrentGeneration,
1265-
InVal.DefInst, &Inst))) {
1288+
InVal.DefInst == getMatchingValue(InVal, MemInst, CurrentGeneration)) {
12661289
// It is okay to have a LastStore to a different pointer here if MemorySSA
12671290
// tells us that the load and store are from the same memory generation.
12681291
// In that case, LastStore should keep its present value since we're

0 commit comments

Comments
 (0)