From: Johannes Doerfert Date: Wed, 4 Sep 2019 16:16:13 +0000 (+0000) Subject: [Attributor] Deal more explicit with non-exact definitions X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=91ae0b70af924659e3f13d05c0e0bea5c2b8f5c9;p=llvm [Attributor] Deal more explicit with non-exact definitions Summary: Before we tried to rule out non-exact definitions early but that lead to on-demand attributes created for them anyway. As a consequence we needed to look at the definition in the initialize of each attribute again. This patch centralized this lookup and tightens the condition under which we give up on non-exact definitions. Reviewers: uenoku, sstefan1 Subscribers: hiraditya, bollu, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D67115 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@370917 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/Transforms/IPO/Attributor.h b/include/llvm/Transforms/IPO/Attributor.h index b511d1125d0..6aa630a4e79 100644 --- a/include/llvm/Transforms/IPO/Attributor.h +++ b/include/llvm/Transforms/IPO/Attributor.h @@ -289,6 +289,19 @@ struct IRPosition { } ///} + /// Return true if the position refers to a function interface, that is the + /// function scope, the function return, or an argumnt. + bool isFnInterfaceKind() const { + switch (getPositionKind()) { + case IRPosition::IRP_FUNCTION: + case IRPosition::IRP_RETURNED: + case IRPosition::IRP_ARGUMENT: + return true; + default: + return false; + } + } + /// Return the Function surrounding the anchor value. /// ///{ @@ -1035,6 +1048,27 @@ struct IRAttribute : public IRPosition, public Base { IRAttribute(const IRPosition &IRP) : IRPosition(IRP) {} ~IRAttribute() {} + /// See AbstractAttribute::initialize(...). + virtual void initialize(Attributor &A) override { + if (hasAttr(getAttrKind())) { + this->getState().indicateOptimisticFixpoint(); + return; + } + + const IRPosition &IRP = this->getIRPosition(); + bool IsFnInterface = IRP.isFnInterfaceKind(); + const Function *FnScope = IRP.getAnchorScope(); + // TODO: Not all attributes require an exact definition. Find a way to + // enable deduction for some but not all attributes in case the + // definition might be changed at runtime, see also + // http://lists.llvm.org/pipermail/llvm-dev/2018-February/121275.html. + // TODO: We could always determine abstract attributes and if sufficient + // information was found we could duplicate the functions that do not + // have an exact definition. + if (IsFnInterface && (!FnScope || !FnScope->hasExactDefinition())) + this->getState().indicatePessimisticFixpoint(); + } + /// See AbstractAttribute::manifest(...). ChangeStatus manifest(Attributor &A) override { SmallVector DeducedAttrs; diff --git a/lib/Transforms/IPO/Attributor.cpp b/lib/Transforms/IPO/Attributor.cpp index 808aae2afd7..580d8e32692 100644 --- a/lib/Transforms/IPO/Attributor.cpp +++ b/lib/Transforms/IPO/Attributor.cpp @@ -646,12 +646,6 @@ struct AACallSiteReturnedFromReturned : public Base { struct AANoUnwindImpl : AANoUnwind { AANoUnwindImpl(const IRPosition &IRP) : AANoUnwind(IRP) {} - /// See AbstractAttribute::initialize(...). - void initialize(Attributor &A) override { - if (hasAttr({Attribute::NoUnwind})) - indicateOptimisticFixpoint(); - } - const std::string getAsStr() const override { return getAssumed() ? "nounwind" : "may-unwind"; } @@ -697,7 +691,7 @@ struct AANoUnwindCallSite final : AANoUnwindImpl { void initialize(Attributor &A) override { AANoUnwindImpl::initialize(A); Function *F = getAssociatedFunction(); - if (!F || !F->hasExactDefinition()) + if (!F) indicatePessimisticFixpoint(); } @@ -757,7 +751,7 @@ public: ReturnedValues.clear(); Function *F = getAssociatedFunction(); - if (!F || !F->hasExactDefinition()) { + if (!F) { indicatePessimisticFixpoint(); return; } @@ -776,6 +770,9 @@ public: return; } } + + if (!F->hasExactDefinition()) + indicatePessimisticFixpoint(); } /// See AbstractAttribute::manifest(...). @@ -1142,12 +1139,6 @@ struct AAReturnedValuesCallSite final : AAReturnedValuesImpl { struct AANoSyncImpl : AANoSync { AANoSyncImpl(const IRPosition &IRP) : AANoSync(IRP) {} - /// See AbstractAttribute::initialize(...). - void initialize(Attributor &A) override { - if (hasAttr({Attribute::NoSync})) - indicateOptimisticFixpoint(); - } - const std::string getAsStr() const override { return getAssumed() ? "nosync" : "may-sync"; } @@ -1315,7 +1306,7 @@ struct AANoSyncCallSite final : AANoSyncImpl { void initialize(Attributor &A) override { AANoSyncImpl::initialize(A); Function *F = getAssociatedFunction(); - if (!F || !F->hasExactDefinition()) + if (!F) indicatePessimisticFixpoint(); } @@ -1341,12 +1332,6 @@ struct AANoSyncCallSite final : AANoSyncImpl { struct AANoFreeImpl : public AANoFree { AANoFreeImpl(const IRPosition &IRP) : AANoFree(IRP) {} - /// See AbstractAttribute::initialize(...). - void initialize(Attributor &A) override { - if (hasAttr({Attribute::NoFree})) - indicateOptimisticFixpoint(); - } - /// See AbstractAttribute::updateImpl(...). ChangeStatus updateImpl(Attributor &A) override { auto CheckForNoFree = [&](Instruction &I) { @@ -1385,7 +1370,7 @@ struct AANoFreeCallSite final : AANoFreeImpl { void initialize(Attributor &A) override { AANoFreeImpl::initialize(A); Function *F = getAssociatedFunction(); - if (!F || !F->hasExactDefinition()) + if (!F) indicatePessimisticFixpoint(); } @@ -1414,6 +1399,8 @@ struct AANonNullImpl : AANonNull { void initialize(Attributor &A) override { if (hasAttr({Attribute::NonNull, Attribute::Dereferenceable})) indicateOptimisticFixpoint(); + else + AANonNull::initialize(A); } /// See AbstractAttribute::getAsStr(). @@ -1519,14 +1506,6 @@ struct AANonNullCallSiteReturned final struct AANoRecurseImpl : public AANoRecurse { AANoRecurseImpl(const IRPosition &IRP) : AANoRecurse(IRP) {} - /// See AbstractAttribute::initialize(...). - void initialize(Attributor &A) override { - if (hasAttr({getAttrKind()})) { - indicateOptimisticFixpoint(); - return; - } - } - /// See AbstractAttribute::getAsStr() const std::string getAsStr() const override { return getAssumed() ? "norecurse" : "may-recurse"; @@ -1553,7 +1532,7 @@ struct AANoRecurseCallSite final : AANoRecurseImpl { void initialize(Attributor &A) override { AANoRecurseImpl::initialize(A); Function *F = getAssociatedFunction(); - if (!F || !F->hasExactDefinition()) + if (!F) indicatePessimisticFixpoint(); } @@ -1606,10 +1585,7 @@ struct AAWillReturnImpl : public AAWillReturn { /// See AbstractAttribute::initialize(...). void initialize(Attributor &A) override { - if (hasAttr({Attribute::WillReturn})) { - indicateOptimisticFixpoint(); - return; - } + AAWillReturn::initialize(A); Function *F = getAssociatedFunction(); if (containsPossiblyEndlessLoop(F)) @@ -1656,7 +1632,7 @@ struct AAWillReturnCallSite final : AAWillReturnImpl { void initialize(Attributor &A) override { AAWillReturnImpl::initialize(A); Function *F = getAssociatedFunction(); - if (!F || !F->hasExactDefinition()) + if (!F) indicatePessimisticFixpoint(); } @@ -1683,12 +1659,6 @@ struct AAWillReturnCallSite final : AAWillReturnImpl { struct AANoAliasImpl : AANoAlias { AANoAliasImpl(const IRPosition &IRP) : AANoAlias(IRP) {} - /// See AbstractAttribute::initialize(...). - void initialize(Attributor &A) override { - if (hasAttr({Attribute::NoAlias})) - indicateOptimisticFixpoint(); - } - const std::string getAsStr() const override { return getAssumed() ? "noalias" : "may-alias"; } @@ -1792,7 +1762,7 @@ struct AANoAliasCallSiteReturned final : AANoAliasImpl { void initialize(Attributor &A) override { AANoAliasImpl::initialize(A); Function *F = getAssociatedFunction(); - if (!F || !F->hasExactDefinition()) + if (!F) indicatePessimisticFixpoint(); } @@ -2186,6 +2156,12 @@ struct AADereferenceableImpl : AADereferenceable { takeKnownDerefBytesMaximum(Attr.getValueAsInt()); NonNullAA = &A.getAAFor(*this, getIRPosition()); + + const IRPosition &IRP = this->getIRPosition(); + bool IsFnInterface = IRP.isFnInterfaceKind(); + const Function *FnScope = IRP.getAnchorScope(); + if (IsFnInterface && (!FnScope || !FnScope->hasExactDefinition())) + indicatePessimisticFixpoint(); } /// See AbstractAttribute::getState() @@ -2340,7 +2316,7 @@ struct AADereferenceableCallSiteReturned final : AADereferenceableImpl { void initialize(Attributor &A) override { AADereferenceableImpl::initialize(A); Function *F = getAssociatedFunction(); - if (!F || !F->hasExactDefinition()) + if (!F) indicatePessimisticFixpoint(); } @@ -2508,7 +2484,7 @@ struct AAAlignCallSiteReturned final : AAAlignImpl { void initialize(Attributor &A) override { AAAlignImpl::initialize(A); Function *F = getAssociatedFunction(); - if (!F || !F->hasExactDefinition()) + if (!F) indicatePessimisticFixpoint(); } @@ -2538,12 +2514,6 @@ struct AANoReturnImpl : public AANoReturn { return getAssumed() ? "noreturn" : "may-return"; } - /// See AbstractAttribute::initialize(...). - void initialize(Attributor &A) override { - if (hasAttr({getAttrKind()})) - indicateOptimisticFixpoint(); - } - /// See AbstractAttribute::updateImpl(Attributor &A). virtual ChangeStatus updateImpl(Attributor &A) override { auto CheckForNoReturn = [](Instruction &) { return false; }; @@ -2569,7 +2539,7 @@ struct AANoReturnCallSite final : AANoReturnImpl { void initialize(Attributor &A) override { AANoReturnImpl::initialize(A); Function *F = getAssociatedFunction(); - if (!F || !F->hasExactDefinition()) + if (!F) indicatePessimisticFixpoint(); } @@ -2599,10 +2569,7 @@ struct AANoCaptureImpl : public AANoCapture { /// See AbstractAttribute::initialize(...). void initialize(Attributor &A) override { - if (hasAttr(Attribute::NoCapture)) { - indicateOptimisticFixpoint(); - return; - } + AANoCapture::initialize(A); const IRPosition &IRP = getIRPosition(); const Function *F = @@ -2611,8 +2578,7 @@ struct AANoCaptureImpl : public AANoCapture { // Check what state the associated function can actually capture. if (F) determineFunctionCaptureCapabilities(*F, *this); - - if (!F || !F->hasExactDefinition()) + else indicatePessimisticFixpoint(); } @@ -2999,7 +2965,7 @@ bool Attributor::checkForAllCallSites(const function_ref &Pred, } CallSite CS(U.getUser()); - if (!CS || !CS.isCallee(&U) || !CS.getCaller()->hasExactDefinition()) { + if (!CS || !CS.isCallee(&U)) { if (!RequireAllCallSites) continue; @@ -3029,7 +2995,7 @@ bool Attributor::checkForAllReturnedValuesAndReturnInsts( // Since we need to provide return instructions we have to have an exact // definition. const Function *AssociatedFunction = IRP.getAssociatedFunction(); - if (!AssociatedFunction || !AssociatedFunction->hasExactDefinition()) + if (!AssociatedFunction) return false; // If this is a call site query we use the call site specific return values @@ -3049,7 +3015,7 @@ bool Attributor::checkForAllReturnedValues( const IRPosition &IRP = QueryingAA.getIRPosition(); const Function *AssociatedFunction = IRP.getAssociatedFunction(); - if (!AssociatedFunction || !AssociatedFunction->hasExactDefinition()) + if (!AssociatedFunction) return false; // TODO: use the function scope once we have call site AAReturnedValues. @@ -3071,7 +3037,7 @@ bool Attributor::checkForAllInstructions( const IRPosition &IRP = QueryingAA.getIRPosition(); // Since we need to provide instructions we have to have an exact definition. const Function *AssociatedFunction = IRP.getAssociatedFunction(); - if (!AssociatedFunction || !AssociatedFunction->hasExactDefinition()) + if (!AssociatedFunction) return false; // TODO: use the function scope once we have call site AAReturnedValues. @@ -3576,25 +3542,16 @@ static bool runAttributorOnModule(Module &M) { Attributor A(InfoCache, DepRecInterval); for (Function &F : M) { - // TODO: Not all attributes require an exact definition. Find a way to - // enable deduction for some but not all attributes in case the - // definition might be changed at runtime, see also - // http://lists.llvm.org/pipermail/llvm-dev/2018-February/121275.html. - // TODO: We could always determine abstract attributes and if sufficient - // information was found we could duplicate the functions that do not - // have an exact definition. - if (!F.hasExactDefinition()) { + if (F.hasExactDefinition()) + NumFnWithExactDefinition++; + else NumFnWithoutExactDefinition++; - continue; - } // For now we ignore naked and optnone functions. if (F.hasFnAttribute(Attribute::Naked) || F.hasFnAttribute(Attribute::OptimizeNone)) continue; - NumFnWithExactDefinition++; - // Populate the Attributor with abstract attribute opportunities in the // function and the information cache with IR information. A.identifyDefaultAbstractAttributes(F); diff --git a/test/Transforms/FunctionAttrs/arg_returned.ll b/test/Transforms/FunctionAttrs/arg_returned.ll index fb7e5d9bc9e..6e57475c579 100644 --- a/test/Transforms/FunctionAttrs/arg_returned.ll +++ b/test/Transforms/FunctionAttrs/arg_returned.ll @@ -796,6 +796,36 @@ r: ret i32 %PHI2 } +define weak_odr i32 @non_exact_0() { + ret i32 0 +} +define weak_odr i32 @non_exact_1(i32 %a) { + ret i32 %a +} +define weak_odr i32 @non_exact_2(i32 returned %a) { + ret i32 %a +} +define weak_odr i32* @non_exact_3(i32* align 32 returned %a) { + ret i32* %a +} +define i32 @exact(i32* %a) { + %c0 = call i32 @non_exact_0() + %c1 = call i32 @non_exact_1(i32 1) + %c2 = call i32 @non_exact_2(i32 2) + %c3 = call i32* @non_exact_3(i32* %a) +; We can use the information of the weak function non_exact_3 because it was +; given to us and not derived (the alignment of the returned argument). +; CHECK: %c4 = load i32, i32* %c3, align 32 + %c4 = load i32, i32* %c3 +; FIXME: %c2 and %c3 should be replaced but not %c0 or %c1! +; CHECK: %add1 = add i32 %c0, %c1 +; CHECK: %add2 = add i32 %add1, %c2 +; CHECK: %add3 = add i32 %add2, %c3 + %add1 = add i32 %c0, %c1 + %add2 = add i32 %add1, %c2 + %add3 = add i32 %add2, %c4 + ret i32 %add3 +} attributes #0 = { noinline nounwind uwtable } diff --git a/test/Transforms/FunctionAttrs/nonnull.ll b/test/Transforms/FunctionAttrs/nonnull.ll index 7a51cb32d6a..69f0b0dd550 100644 --- a/test/Transforms/FunctionAttrs/nonnull.ll +++ b/test/Transforms/FunctionAttrs/nonnull.ll @@ -458,4 +458,16 @@ define i32* @g1() { ret i32* %c } +; ATTRIBUTOR: define internal void @called_by_weak(i32* nocapture nonnull %a) +define internal void @called_by_weak(i32* %a) { + ret void +} + +; Check we do not annotate the function interface of this weak function. +; ATTRIBUTOR: define weak_odr void @weak_caller(i32* nonnull %a) +define weak_odr void @weak_caller(i32* nonnull %a) { + call void @called_by_weak(i32* %a) + ret void +} + attributes #0 = { "null-pointer-is-valid"="true" } diff --git a/test/Transforms/FunctionAttrs/noreturn_async.ll b/test/Transforms/FunctionAttrs/noreturn_async.ll index b7e9b0f4058..57174d4d90d 100644 --- a/test/Transforms/FunctionAttrs/noreturn_async.ll +++ b/test/Transforms/FunctionAttrs/noreturn_async.ll @@ -1,4 +1,4 @@ -; RUN: opt -functionattrs -attributor -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=3 -S < %s | FileCheck %s +; RUN: opt -functionattrs -attributor -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=4 -S < %s | FileCheck %s ; ; This file is the same as noreturn_sync.ll but with a personality which ; indicates that the exception handler *can* catch asynchronous exceptions. As diff --git a/test/Transforms/FunctionAttrs/nounwind.ll b/test/Transforms/FunctionAttrs/nounwind.ll index 50a229dcb3d..495aaeaacac 100644 --- a/test/Transforms/FunctionAttrs/nounwind.ll +++ b/test/Transforms/FunctionAttrs/nounwind.ll @@ -1,5 +1,5 @@ ; RUN: opt < %s -functionattrs -S | FileCheck %s -; RUN: opt < %s -attributor -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=4 -S | FileCheck %s --check-prefix=ATTRIBUTOR +; RUN: opt < %s -attributor -attributor-disable=false -attributor-max-iterations-verify -attributor-max-iterations=6 -S | FileCheck %s --check-prefix=ATTRIBUTOR ; TEST 1 ; CHECK: Function Attrs: norecurse nounwind readnone