Skip to content

Commit

Permalink
[GVN] Refactor handling of pointer-select in GVN pass
Browse files Browse the repository at this point in the history 
This patch extends Def memory dependency with support of select
instructions to consistently handle pointer-select conversion.

Differential Revision: https://reviews.llvm.org/D141619
  • Loading branch information
@skachkov-sc
skachkov-sc committed Jan 17, 2023
1 parent 5091357 commit bfd2dd4
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 49 deletions.
8 changes: 8 additions & 0 deletions llvm/include/llvm/Analysis/MemoryDependenceAnalysis.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,11 @@ class MemDepResult {
/// calls or memory use intrinsics with identical callees and no
/// intervening clobbers. No validation is done that the operands to
/// the calls are the same.
/// 4. For loads and stores, this could be a select instruction that
/// defines pointer to this memory location. In this case, users can
/// find non-clobbered Defs for both select values that are reaching
// the desired memory location (there is still a guarantee that there
// are no clobbers between analyzed memory location and select).
Def,

/// This marker indicates that the query has no known dependency in the
Expand Down Expand Up @@ -142,6 +147,9 @@ class MemDepResult {
/// definition dependency.
bool isDef() const { return Value.is<Def>(); }

/// Tests if this MemDepResult represents a valid local query (Clobber/Def).
bool isLocal() const { return isClobber() || isDef(); }

/// Tests if this MemDepResult represents a query that is transparent to the
/// start of the block, but where a non-local hasn't been done.
bool isNonLocal() const {
Expand Down
7 changes: 6 additions & 1 deletion llvm/lib/Analysis/MemoryDependenceAnalysis.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,11 @@ MemDepResult MemoryDependenceResults::getSimplePointerDependencyFrom(
return MemDepResult::getDef(Inst);
}

// If we found a select instruction for MemLoc pointer, return it as Def
// dependency.
if (isa<SelectInst>(Inst) && MemLoc.Ptr == Inst)
return MemDepResult::getDef(Inst);

if (isInvariantLoad)
continue;

Expand Down Expand Up @@ -962,7 +967,7 @@ MemDepResult MemoryDependenceResults::getNonLocalInfoForBlock(
// If the block has a dependency (i.e. it isn't completely transparent to
// the value), remember the reverse association because we just added it
// to Cache!
if (!Dep.isDef() && !Dep.isClobber())
if (!Dep.isLocal())
return Dep;

// Keep the ReverseNonLocalPtrDeps map up to date so we can efficiently
Expand Down
66 changes: 25 additions & 41 deletions llvm/lib/Transforms/Scalar/GVN.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1120,31 +1120,26 @@ static void reportMayClobberedLoad(LoadInst *Load, MemDepResult DepInfo,
/// 1. The pointer select (\p Address) must be defined in \p DepBB.
/// 2. Both value operands of the pointer select must be loaded in the same
/// basic block, before the pointer select.
/// 3. There must be no instructions between the found loads and \p End that may
/// 3. There must be no instructions between the found loads and \p Sel that may
/// clobber the loads.
static std::optional<AvailableValue>
tryToConvertLoadOfPtrSelect(BasicBlock *DepBB, BasicBlock::iterator End,
Value *Address, Type *LoadTy, DominatorTree &DT,
tryToConvertLoadOfPtrSelect(SelectInst *Sel, Type *LoadTy, DominatorTree &DT,
AAResults *AA) {

auto *Sel = dyn_cast_or_null<SelectInst>(Address);
if (!Sel || DepBB != Sel->getParent())
return std::nullopt;

LoadInst *L1 = findDominatingLoad(Sel->getOperand(1), LoadTy, Sel, DT);
LoadInst *L2 = findDominatingLoad(Sel->getOperand(2), LoadTy, Sel, DT);
if (!L1 || !L2)
return std::nullopt;

// Ensure there are no accesses that may modify the locations referenced by
// either L1 or L2 between L1, L2 and the specified End iterator.
// either L1 or L2 between L1, L2 and the specified Sel instruction.
Instruction *EarlierLoad = L1->comesBefore(L2) ? L1 : L2;
MemoryLocation L1Loc = MemoryLocation::get(L1);
MemoryLocation L2Loc = MemoryLocation::get(L2);
if (any_of(make_range(EarlierLoad->getIterator(), End), [&](Instruction &I) {
return isModSet(AA->getModRefInfo(&I, L1Loc)) ||
isModSet(AA->getModRefInfo(&I, L2Loc));
}))
if (any_of(make_range(EarlierLoad->getIterator(), Sel->getIterator()),
[&](Instruction &I) {
return isModSet(AA->getModRefInfo(&I, L1Loc)) ||
isModSet(AA->getModRefInfo(&I, L2Loc));
}))
return std::nullopt;

return AvailableValue::getSelect(Sel);
Expand All @@ -1153,20 +1148,12 @@ tryToConvertLoadOfPtrSelect(BasicBlock *DepBB, BasicBlock::iterator End,
std::optional<AvailableValue>
GVNPass::AnalyzeLoadAvailability(LoadInst *Load, MemDepResult DepInfo,
Value *Address) {
if (!DepInfo.isDef() && !DepInfo.isClobber()) {
assert(isa<SelectInst>(Address));
return tryToConvertLoadOfPtrSelect(Load->getParent(), Load->getIterator(),
Address, Load->getType(),
getDominatorTree(), getAliasAnalysis());
}

assert((DepInfo.isDef() || DepInfo.isClobber()) &&
"expected a local dependence");
assert(Load->isUnordered() && "rules below are incorrect for ordered access");

const DataLayout &DL = Load->getModule()->getDataLayout();
assert(DepInfo.isLocal() && "expected a local dependence");

Instruction *DepInst = DepInfo.getInst();

const DataLayout &DL = Load->getModule()->getDataLayout();
if (DepInfo.isClobber()) {
// If the dependence is to a store that writes to a superset of the bits
// read by the load, we can extract the bits we need for the load from the
Expand Down Expand Up @@ -1272,6 +1259,13 @@ GVNPass::AnalyzeLoadAvailability(LoadInst *Load, MemDepResult DepInfo,
return AvailableValue::getLoad(LD);
}

// Check if load with Addr dependent from select can be converted to select
// between load values. There must be no instructions between the found
// loads and DepInst that may clobber the loads.
if (auto *Sel = dyn_cast<SelectInst>(DepInst))
return tryToConvertLoadOfPtrSelect(Sel, Load->getType(), getDominatorTree(),
getAliasAnalysis());

// Unknown def - must be conservative
LLVM_DEBUG(
// fast print dep, using operator<< on instruction is too slow.
Expand All @@ -1298,24 +1292,15 @@ void GVNPass::AnalyzeLoadAvailability(LoadInst *Load, LoadDepVect &Deps,
continue;
}

// The address being loaded in this non-local block may not be the same as
// the pointer operand of the load if PHI translation occurs. Make sure
// to consider the right address.
Value *Address = Dep.getAddress();

if (!DepInfo.isDef() && !DepInfo.isClobber()) {
if (auto R = tryToConvertLoadOfPtrSelect(
DepBB, DepBB->end(), Address, Load->getType(), getDominatorTree(),
getAliasAnalysis())) {
ValuesPerBlock.push_back(
AvailableValueInBlock::get(DepBB, std::move(*R)));
continue;
}
if (!DepInfo.isLocal()) {
UnavailableBlocks.push_back(DepBB);
continue;
}

if (auto AV = AnalyzeLoadAvailability(Load, DepInfo, Address)) {
// The address being loaded in this non-local block may not be the same as
// the pointer operand of the load if PHI translation occurs. Make sure
// to consider the right address.
if (auto AV = AnalyzeLoadAvailability(Load, DepInfo, Dep.getAddress())) {
// subtlety: because we know this was a non-local dependency, we know
// it's safe to materialize anywhere between the instruction within
// DepInfo and the end of it's block.
Expand Down Expand Up @@ -2043,9 +2028,8 @@ bool GVNPass::processLoad(LoadInst *L) {
if (Dep.isNonLocal())
return processNonLocalLoad(L);

Value *Address = L->getPointerOperand();
// Only handle the local case below
if (!Dep.isDef() && !Dep.isClobber() && !isa<SelectInst>(Address)) {
if (!Dep.isLocal()) {
// This might be a NonFuncLocal or an Unknown
LLVM_DEBUG(
// fast print dep, using operator<< on instruction is too slow.
Expand All @@ -2054,7 +2038,7 @@ bool GVNPass::processLoad(LoadInst *L) {
return false;
}

auto AV = AnalyzeLoadAvailability(L, Dep, Address);
auto AV = AnalyzeLoadAvailability(L, Dep, L->getPointerOperand());
if (!AV)
return false;

Expand Down
4 changes: 3 additions & 1 deletion llvm/test/Transforms/GVN/PRE/pre-load-through-select.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -236,13 +236,15 @@ define i32 @test_pointer_phi_select_simp_store_clobber_3(ptr %a, ptr %b, ptr %c,
; CHECK-NEXT: [[L_1:%.*]] = load i32, ptr [[A:%.*]], align 4
; CHECK-NEXT: [[L_2:%.*]] = load i32, ptr [[B:%.*]], align 4
; CHECK-NEXT: [[CMP_I_I_I:%.*]] = icmp ult i32 [[L_1]], [[L_2]]
; CHECK-NEXT: [[TMP0:%.*]] = select i1 [[CMP_I_I_I]], i32 [[L_1]], i32 [[L_2]]
; CHECK-NEXT: [[MIN_SELECT:%.*]] = select i1 [[CMP_I_I_I]], ptr [[A]], ptr [[B]]
; CHECK-NEXT: br label [[EXIT:%.*]]
; CHECK: else:
; CHECK-NEXT: [[RES_2_PRE:%.*]] = load i32, ptr [[A]], align 4
; CHECK-NEXT: br label [[EXIT]]
; CHECK: exit:
; CHECK-NEXT: [[RES_2:%.*]] = phi i32 [ [[TMP0]], [[THEN]] ], [ [[RES_2_PRE]], [[ELSE]] ]
; CHECK-NEXT: [[P:%.*]] = phi ptr [ [[MIN_SELECT]], [[THEN]] ], [ [[A]], [[ELSE]] ]
; CHECK-NEXT: [[RES_2:%.*]] = load i32, ptr [[P]], align 4
; CHECK-NEXT: ret i32 [[RES_2]]
;
entry:
Expand Down
12 changes: 6 additions & 6 deletions llvm/test/Transforms/GVN/PRE/pre-loop-load-through-select.ll
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,13 +311,13 @@ define i32 @test_pointer_phi_select_load_may_not_execute_1(ptr %A, ptr %B, ptr %
; CHECK-NEXT: [[L_1:%.*]] = load i32, ptr [[PTR_IV]], align 4
; CHECK-NEXT: [[L_2:%.*]] = load i32, ptr [[MIN_PTR]], align 4
; CHECK-NEXT: [[CMP_I_I_I:%.*]] = icmp ult i32 [[L_1]], [[L_2]]
; CHECK-NEXT: [[TMP0:%.*]] = select i1 [[CMP_I_I_I]], i32 [[L_1]], i32 [[L_2]]
; CHECK-NEXT: [[MIN_SELECT]] = select i1 [[CMP_I_I_I]], ptr [[PTR_IV]], ptr [[MIN_PTR]]
; CHECK-NEXT: [[PTR_IV_NEXT]] = getelementptr inbounds i32, ptr [[PTR_IV]], i64 1
; CHECK-NEXT: [[EC:%.*]] = icmp eq ptr [[PTR_IV_NEXT]], [[END:%.*]]
; CHECK-NEXT: br i1 [[EC]], label [[EXIT:%.*]], label [[LOOP]]
; CHECK: exit:
; CHECK-NEXT: [[RES:%.*]] = load i32, ptr [[MIN_SELECT]], align 4
; CHECK-NEXT: ret i32 [[RES]]
; CHECK-NEXT: ret i32 [[TMP0]]
;
entry:
br label %loop
Expand Down Expand Up @@ -350,13 +350,13 @@ define i32 @test_pointer_phi_select_load_may_not_execute_2(ptr %A, ptr %B, ptr %
; CHECK-NEXT: call void @may_throw()
; CHECK-NEXT: [[L_2:%.*]] = load i32, ptr [[MIN_PTR]], align 4
; CHECK-NEXT: [[CMP_I_I_I:%.*]] = icmp ult i32 [[L_1]], [[L_2]]
; CHECK-NEXT: [[TMP0:%.*]] = select i1 [[CMP_I_I_I]], i32 [[L_1]], i32 [[L_2]]
; CHECK-NEXT: [[MIN_SELECT]] = select i1 [[CMP_I_I_I]], ptr [[PTR_IV]], ptr [[MIN_PTR]]
; CHECK-NEXT: [[PTR_IV_NEXT]] = getelementptr inbounds i32, ptr [[PTR_IV]], i64 1
; CHECK-NEXT: [[EC:%.*]] = icmp eq ptr [[PTR_IV_NEXT]], [[END:%.*]]
; CHECK-NEXT: br i1 [[EC]], label [[EXIT:%.*]], label [[LOOP]]
; CHECK: exit:
; CHECK-NEXT: [[RES:%.*]] = load i32, ptr [[MIN_SELECT]], align 4
; CHECK-NEXT: ret i32 [[RES]]
; CHECK-NEXT: ret i32 [[TMP0]]
;
entry:
br label %loop
Expand Down Expand Up @@ -514,13 +514,13 @@ define i32 @test_pointer_phi_select_same_object_may_write_call_1(ptr %ptr, ptr %
; CHECK-NEXT: [[L_1:%.*]] = load i32, ptr [[PTR_IV]], align 4
; CHECK-NEXT: [[L_2:%.*]] = load i32, ptr [[MIN_PTR]], align 4
; CHECK-NEXT: [[CMP_I_I_I:%.*]] = icmp ult i32 [[L_1]], [[L_2]]
; CHECK-NEXT: [[TMP0:%.*]] = select i1 [[CMP_I_I_I]], i32 [[L_1]], i32 [[L_2]]
; CHECK-NEXT: [[MIN_SELECT]] = select i1 [[CMP_I_I_I]], ptr [[PTR_IV]], ptr [[MIN_PTR]]
; CHECK-NEXT: [[PTR_IV_NEXT]] = getelementptr inbounds i32, ptr [[PTR_IV]], i64 1
; CHECK-NEXT: [[EC:%.*]] = icmp eq ptr [[PTR_IV_NEXT]], [[END:%.*]]
; CHECK-NEXT: br i1 [[EC]], label [[EXIT:%.*]], label [[LOOP]]
; CHECK: exit:
; CHECK-NEXT: [[RES:%.*]] = load i32, ptr [[MIN_SELECT]], align 4
; CHECK-NEXT: ret i32 [[RES]]
; CHECK-NEXT: ret i32 [[TMP0]]
;
entry:
%start.ptr = getelementptr inbounds i32, ptr %ptr, i64 1
Expand Down

0 comments on commit bfd2dd4

Please sign in to comment.