Skip to content

Commit b3fec10

Browse files
jdoerfertjdoerfert
committed
[Attributor] Improve NonNull deduction
We can improve our deduction if we stop at PHI and select instructions and also iterate the returned values explicitly. The latter helps with isImpliedByIR deductions.
1 parent be22b90 commit b3fec10

File tree

6 files changed

+60
-25
lines changed

6 files changed

+60
-25
lines changed

‎llvm/include/llvm/Transforms/IPO/Attributor.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2289,7 +2289,7 @@ struct Attributor {
22892289
/// present in \p Opcode and return true if \p Pred holds on all of them.
22902290
bool checkForAllInstructions(function_ref<bool(Instruction &)> Pred,
22912291
const Function *Fn,
2292-
const AbstractAttribute &QueryingAA,
2292+
const AbstractAttribute *QueryingAA,
22932293
const ArrayRef<unsigned> &Opcodes,
22942294
bool &UsedAssumedInformation,
22952295
bool CheckBBLivenessOnly = false,

‎llvm/lib/Transforms/IPO/Attributor.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -723,7 +723,7 @@ isPotentiallyReachable(Attributor &A, const Instruction &FromI,
723723

724724
// Check if we can reach returns.
725725
bool UsedAssumedInformation = false;
726-
if (A.checkForAllInstructions(ReturnInstCB, FromFn, QueryingAA,
726+
if (A.checkForAllInstructions(ReturnInstCB, FromFn, &QueryingAA,
727727
{Instruction::Ret}, UsedAssumedInformation)) {
728728
LLVM_DEBUG(dbgs() << "[AA] No return is reachable, done\n");
729729
continue;
@@ -1967,7 +1967,7 @@ static bool checkForAllInstructionsImpl(
19671967

19681968
bool Attributor::checkForAllInstructions(function_ref<bool(Instruction &)> Pred,
19691969
const Function *Fn,
1970-
const AbstractAttribute &QueryingAA,
1970+
const AbstractAttribute *QueryingAA,
19711971
const ArrayRef<unsigned> &Opcodes,
19721972
bool &UsedAssumedInformation,
19731973
bool CheckBBLivenessOnly,
@@ -1978,12 +1978,12 @@ bool Attributor::checkForAllInstructions(function_ref<bool(Instruction &)> Pred,
19781978

19791979
const IRPosition &QueryIRP = IRPosition::function(*Fn);
19801980
const auto *LivenessAA =
1981-
CheckPotentiallyDead
1981+
CheckPotentiallyDead && QueryingAA
19821982
? nullptr
1983-
: (getAAFor<AAIsDead>(QueryingAA, QueryIRP, DepClassTy::NONE));
1983+
: (getAAFor<AAIsDead>(*QueryingAA, QueryIRP, DepClassTy::NONE));
19841984

19851985
auto &OpcodeInstMap = InfoCache.getOpcodeInstMapForFunction(*Fn);
1986-
if (!checkForAllInstructionsImpl(this, OpcodeInstMap, Pred, &QueryingAA,
1986+
if (!checkForAllInstructionsImpl(this, OpcodeInstMap, Pred, QueryingAA,
19871987
LivenessAA, Opcodes, UsedAssumedInformation,
19881988
CheckBBLivenessOnly, CheckPotentiallyDead))
19891989
return false;
@@ -1999,7 +1999,7 @@ bool Attributor::checkForAllInstructions(function_ref<bool(Instruction &)> Pred,
19991999
bool CheckPotentiallyDead) {
20002000
const IRPosition &IRP = QueryingAA.getIRPosition();
20012001
const Function *AssociatedFunction = IRP.getAssociatedFunction();
2002-
return checkForAllInstructions(Pred, AssociatedFunction, QueryingAA, Opcodes,
2002+
return checkForAllInstructions(Pred, AssociatedFunction, &QueryingAA, Opcodes,
20032003
UsedAssumedInformation, CheckBBLivenessOnly,
20042004
CheckPotentiallyDead);
20052005
}

‎llvm/lib/Transforms/IPO/AttributorAttributes.cpp

Lines changed: 40 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2457,9 +2457,6 @@ bool AANonNull::isImpliedByIR(Attributor &A, const IRPosition &IRP,
24572457
if (A.hasAttr(IRP, AttrKinds, IgnoreSubsumingPositions, Attribute::NonNull))
24582458
return true;
24592459

2460-
if (IRP.getPositionKind() == IRP_RETURNED)
2461-
return false;
2462-
24632460
DominatorTree *DT = nullptr;
24642461
AssumptionCache *AC = nullptr;
24652462
InformationCache &InfoCache = A.getInfoCache();
@@ -2470,9 +2467,27 @@ bool AANonNull::isImpliedByIR(Attributor &A, const IRPosition &IRP,
24702467
}
24712468
}
24722469

2473-
if (!isKnownNonZero(&IRP.getAssociatedValue(), A.getDataLayout(), 0, AC,
2474-
IRP.getCtxI(), DT))
2470+
SmallVector<AA::ValueAndContext> Worklist;
2471+
if (IRP.getPositionKind() != IRP_RETURNED) {
2472+
Worklist.push_back({IRP.getAssociatedValue(), IRP.getCtxI()});
2473+
} else {
2474+
bool UsedAssumedInformation = false;
2475+
if (!A.checkForAllInstructions(
2476+
[&](Instruction &I) {
2477+
Worklist.push_back({*cast<ReturnInst>(I).getReturnValue(), &I});
2478+
return true;
2479+
},
2480+
IRP.getAssociatedFunction(), nullptr, {Instruction::Ret},
2481+
UsedAssumedInformation))
2482+
return false;
2483+
}
2484+
2485+
if (llvm::any_of(Worklist, [&](AA::ValueAndContext VAC) {
2486+
return !isKnownNonZero(VAC.getValue(), A.getDataLayout(), 0, AC,
2487+
VAC.getCtxI(), DT);
2488+
}))
24752489
return false;
2490+
24762491
A.manifestAttrs(IRP, {Attribute::get(IRP.getAnchorValue().getContext(),
24772492
Attribute::NonNull)});
24782493
return true;
@@ -2617,6 +2632,23 @@ struct AANonNullFloating : public AANonNullImpl {
26172632
Values.size() != 1 || Values.front().getValue() != AssociatedValue;
26182633

26192634
if (!Stripped) {
2635+
bool IsKnown;
2636+
if (auto *PHI = dyn_cast<PHINode>(AssociatedValue))
2637+
if (llvm::all_of(PHI->incoming_values(), [&](Value *Op) {
2638+
return AA::hasAssumedIRAttr<Attribute::NonNull>(
2639+
A, this, IRPosition::value(*Op), DepClassTy::OPTIONAL,
2640+
IsKnown);
2641+
}))
2642+
return ChangeStatus::UNCHANGED;
2643+
if (auto *Select = dyn_cast<SelectInst>(AssociatedValue))
2644+
if (AA::hasAssumedIRAttr<Attribute::NonNull>(
2645+
A, this, IRPosition::value(*Select->getFalseValue()),
2646+
DepClassTy::OPTIONAL, IsKnown) &&
2647+
AA::hasAssumedIRAttr<Attribute::NonNull>(
2648+
A, this, IRPosition::value(*Select->getTrueValue()),
2649+
DepClassTy::OPTIONAL, IsKnown))
2650+
return ChangeStatus::UNCHANGED;
2651+
26202652
// If we haven't stripped anything we might still be able to use a
26212653
// different AA, but only if the IRP changes. Effectively when we
26222654
// interpret this not as a call site value but as a floating/argument
@@ -2641,10 +2673,11 @@ struct AANonNullFloating : public AANonNullImpl {
26412673
/// NonNull attribute for function return value.
26422674
struct AANonNullReturned final
26432675
: AAReturnedFromReturnedValues<AANonNull, AANonNull, AANonNull::StateType,
2644-
false, AANonNull::IRAttributeKind> {
2676+
false, AANonNull::IRAttributeKind, false> {
26452677
AANonNullReturned(const IRPosition &IRP, Attributor &A)
26462678
: AAReturnedFromReturnedValues<AANonNull, AANonNull, AANonNull::StateType,
2647-
false, Attribute::NonNull>(IRP, A) {}
2679+
false, Attribute::NonNull, false>(IRP, A) {
2680+
}
26482681

26492682
/// See AbstractAttribute::getAsStr().
26502683
const std::string getAsStr(Attributor *A) const override {

‎llvm/test/Transforms/Attributor/depgraph.ll

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ define ptr @checkAndAdvance(ptr align 16 %0) {
112112
; GRAPH-NEXT: updates [AAPotentialValues] for CtxI ' %6 = call ptr @checkAndAdvance(ptr %5)' at position {cs_ret: [@-1]} with state set-state(< { %5 = getelementptr inbounds i32, ptr %0, i64 4[3], %5 = getelementptr inbounds i32, ptr %0, i64 4[3], } >)
113113
; GRAPH-NEXT: updates [AANoCapture] for CtxI ' %2 = load i32, ptr %0, align 4' at position {arg: [@0]} with state assumed not-captured-maybe-returned
114114
; GRAPH-NEXT: updates [AAAlign] for CtxI ' %2 = load i32, ptr %0, align 4' at position {fn_ret:checkAndAdvance [checkAndAdvance@-1]} with state align<1-16>
115-
; GRAPH-NEXT: updates [AANonNull] for CtxI ' %2 = load i32, ptr %0, align 4' at position {fn_ret:checkAndAdvance [checkAndAdvance@-1]} with state nonnull
115+
; GRAPH-NEXT: updates [AANonNull] for CtxI ' %.0 = phi ptr [ %6, %4 ], [ %0, %7 ]' at position {flt:.0 [.0@-1]} with state nonnull
116116
; GRAPH-EMPTY:
117117
; GRAPH-NEXT: [AAPotentialValues] for CtxI ' %5 = getelementptr inbounds i32, ptr %0, i64 4' at position {flt: [@-1]} with state set-state(< { %5 = getelementptr inbounds i32, ptr %0, i64 4[3], } >)
118118
; GRAPH-EMPTY:
@@ -207,6 +207,9 @@ define ptr @checkAndAdvance(ptr align 16 %0) {
207207
; GRAPH-EMPTY:
208208
; GRAPH-NEXT: [AANonNull] for CtxI ' %2 = load i32, ptr %0, align 4' at position {fn_ret:checkAndAdvance [checkAndAdvance@-1]} with state nonnull
209209
; GRAPH-EMPTY:
210+
; GRAPH-NEXT: [AANonNull] for CtxI ' %.0 = phi ptr [ %6, %4 ], [ %0, %7 ]' at position {flt:.0 [.0@-1]} with state nonnull
211+
; GRAPH-NEXT: updates [AANonNull] for CtxI ' %2 = load i32, ptr %0, align 4' at position {fn_ret:checkAndAdvance [checkAndAdvance@-1]} with state nonnull
212+
; GRAPH-EMPTY:
210213
; GRAPH-NEXT: [AANonNull] for CtxI ' %2 = load i32, ptr %0, align 4' at position {arg: [@0]} with state nonnull
211214
; GRAPH-EMPTY:
212215
; GRAPH-NEXT: [AANoAlias] for CtxI ' %2 = load i32, ptr %0, align 4' at position {fn_ret:checkAndAdvance [checkAndAdvance@-1]} with state may-alias

‎llvm/test/Transforms/Attributor/nonnull.ll

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1522,7 +1522,6 @@ define void @nonnull_caller(ptr %p) {
15221522
call void @nonnull_callee(ptr %p)
15231523
ret void
15241524
}
1525-
15261525
declare void @nonnull_callee(ptr nonnull %p)
15271526

15281527
define ptr @phi(ptr %p) {
@@ -1555,15 +1554,15 @@ define void @phi_caller(ptr %p) {
15551554
; TUNIT: Function Attrs: nounwind
15561555
; TUNIT-LABEL: define {{[^@]+}}@phi_caller
15571556
; TUNIT-SAME: (ptr nofree [[P:%.*]]) #[[ATTR5]] {
1558-
; TUNIT-NEXT: [[C:%.*]] = call ptr @phi(ptr noalias nofree readnone [[P]]) #[[ATTR19:[0-9]+]]
1559-
; TUNIT-NEXT: call void @use_i8_ptr(ptr noalias nocapture nofree readnone [[C]]) #[[ATTR5]]
1557+
; TUNIT-NEXT: [[C:%.*]] = call nonnull ptr @phi(ptr noalias nofree readnone [[P]]) #[[ATTR19:[0-9]+]]
1558+
; TUNIT-NEXT: call void @use_i8_ptr(ptr noalias nocapture nofree nonnull readnone [[C]]) #[[ATTR5]]
15601559
; TUNIT-NEXT: ret void
15611560
;
15621561
; CGSCC: Function Attrs: nounwind
15631562
; CGSCC-LABEL: define {{[^@]+}}@phi_caller
15641563
; CGSCC-SAME: (ptr nofree [[P:%.*]]) #[[ATTR4]] {
1565-
; CGSCC-NEXT: [[C:%.*]] = call ptr @phi(ptr noalias nofree readnone [[P]]) #[[ATTR15]]
1566-
; CGSCC-NEXT: call void @use_i8_ptr(ptr noalias nocapture nofree readnone [[C]]) #[[ATTR4]]
1564+
; CGSCC-NEXT: [[C:%.*]] = call nonnull ptr @phi(ptr noalias nofree readnone [[P]]) #[[ATTR15]]
1565+
; CGSCC-NEXT: call void @use_i8_ptr(ptr noalias nocapture nofree nonnull readnone [[C]]) #[[ATTR4]]
15671566
; CGSCC-NEXT: ret void
15681567
;
15691568
%c = call ptr @phi(ptr %p)
@@ -1595,15 +1594,15 @@ define void @multi_ret_caller(ptr %p) {
15951594
; TUNIT: Function Attrs: nounwind
15961595
; TUNIT-LABEL: define {{[^@]+}}@multi_ret_caller
15971596
; TUNIT-SAME: (ptr nofree [[P:%.*]]) #[[ATTR5]] {
1598-
; TUNIT-NEXT: [[C:%.*]] = call ptr @multi_ret(ptr noalias nofree readnone [[P]]) #[[ATTR19]]
1599-
; TUNIT-NEXT: call void @use_i8_ptr(ptr noalias nocapture nofree readnone [[C]]) #[[ATTR5]]
1597+
; TUNIT-NEXT: [[C:%.*]] = call nonnull ptr @multi_ret(ptr noalias nofree readnone [[P]]) #[[ATTR19]]
1598+
; TUNIT-NEXT: call void @use_i8_ptr(ptr noalias nocapture nofree nonnull readnone [[C]]) #[[ATTR5]]
16001599
; TUNIT-NEXT: ret void
16011600
;
16021601
; CGSCC: Function Attrs: nounwind
16031602
; CGSCC-LABEL: define {{[^@]+}}@multi_ret_caller
16041603
; CGSCC-SAME: (ptr nofree [[P:%.*]]) #[[ATTR4]] {
1605-
; CGSCC-NEXT: [[C:%.*]] = call ptr @multi_ret(ptr noalias nofree readnone [[P]]) #[[ATTR15]]
1606-
; CGSCC-NEXT: call void @use_i8_ptr(ptr noalias nocapture nofree readnone [[C]]) #[[ATTR4]]
1604+
; CGSCC-NEXT: [[C:%.*]] = call nonnull ptr @multi_ret(ptr noalias nofree readnone [[P]]) #[[ATTR15]]
1605+
; CGSCC-NEXT: call void @use_i8_ptr(ptr noalias nocapture nofree nonnull readnone [[C]]) #[[ATTR4]]
16071606
; CGSCC-NEXT: ret void
16081607
;
16091608
%c = call ptr @multi_ret(ptr %p)

‎llvm/test/Transforms/Attributor/value-simplify.ll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ define ptr @complicated_args_preallocated() {
502502
; CGSCC-SAME: () #[[ATTR4:[0-9]+]] {
503503
; CGSCC-NEXT: [[C:%.*]] = call token @llvm.call.preallocated.setup(i32 noundef 1) #[[ATTR13]]
504504
; CGSCC-NEXT: [[CALL:%.*]] = call ptr @test_preallocated(ptr nofree noundef writeonly preallocated(i32) align 4294967296 null) #[[ATTR14:[0-9]+]] [ "preallocated"(token [[C]]) ]
505-
; CGSCC-NEXT: ret ptr null
505+
; CGSCC-NEXT: unreachable
506506
;
507507
%c = call token @llvm.call.preallocated.setup(i32 1)
508508
%call = call ptr @test_preallocated(ptr preallocated(i32) null) ["preallocated"(token %c)]

0 commit comments

Comments
 (0)