[llvm] 8cf1cc5 - [FuncAttrs] Infer noreturn
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Thu Jan 7 09:28:17 PST 2021
On Thu, Jan 7, 2021 at 6:07 PM Philip Reames via llvm-commits <
llvm-commits at lists.llvm.org> wrote:
> Artur,
>
> I see a serious factoring problem with this change. Specifically,
> existing passes (instcombine, simplifycfg) should have pruned
> instructions after noreturn calls leaving only a unreachable
> terminator. Given that, scanning the terminator block should not be
> required at all here.
>
> Are you sure your motivating test requires handling noreturn calls
> explicitly? (As opposed to simply checking for a lack of return
> instructions?)
>
> The only solid argument I see for handling no return calls is analysis
> of SCCs, but your patch doesn't appear to do that, so it's no relevant.
> Unless you're planning to support that in the near future?
>
> At the moment, I am not asking for a revert. Depending on your answer
> and resulting discussion, I may.
>
> Philip
>
I think the problem here might be that under the NewPM, SimplifyCFG (which
does the noreturn unreachable fold) does not run before FunctionAttrs. On
the legacy PM it does.
Nikita
> On 1/5/21 1:26 PM, Arthur Eubanks via llvm-commits wrote:
> > Author: Arthur Eubanks
> > Date: 2021-01-05T13:25:42-08:00
> > New Revision: 8cf1cc578d3288f5b9cfe1584e524f8b1517dc97
> >
> > URL:
> https://github.com/llvm/llvm-project/commit/8cf1cc578d3288f5b9cfe1584e524f8b1517dc97
> > DIFF:
> https://github.com/llvm/llvm-project/commit/8cf1cc578d3288f5b9cfe1584e524f8b1517dc97.diff
> >
> > LOG: [FuncAttrs] Infer noreturn
> >
> > A function is noreturn if all blocks terminating with a ReturnInst
> > contain a call to a noreturn function. Skip looking at naked functions
> > since there may be asm that returns.
> >
> > This can be further refined in the future by checking unreachable blocks
> > and taking into account recursion. It looks like the attributor pass
> > does this, but that is not yet enabled by default.
> >
> > This seems to help with code size under the new PM since PruneEH does
> > not run under the new PM, missing opportunities to mark some functions
> > noreturn, which in turn doesn't allow simplifycfg to clean up dead code.
> > https://bugs.llvm.org/show_bug.cgi?id=46858.
> >
> > Reviewed By: rnk
> >
> > Differential Revision: https://reviews.llvm.org/D93946
> >
> > Added:
> > llvm/test/Transforms/FunctionAttrs/noreturn.ll
> >
> > Modified:
> > llvm/lib/Transforms/IPO/FunctionAttrs.cpp
> > llvm/test/Transforms/PruneEH/simplenoreturntest.ll
> >
> > Removed:
> >
> >
> >
> >
> ################################################################################
> > diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
> b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
> > index 0d25d5d319e7..62e3e5ad0a52 100644
> > --- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
> > +++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp
> > @@ -1388,6 +1388,42 @@ static bool addNoRecurseAttrs(const SCCNodeSet
> &SCCNodes) {
> > return true;
> > }
> >
> > +static bool instructionDoesNotReturn(Instruction &I) {
> > + if (auto *CB = dyn_cast<CallBase>(&I)) {
> > + Function *Callee = CB->getCalledFunction();
> > + return Callee && Callee->doesNotReturn();
> > + }
> > + return false;
> > +}
> > +
> > +// A basic block can only return if it terminates with a ReturnInst and
> does not
> > +// contain calls to noreturn functions.
> > +static bool basicBlockCanReturn(BasicBlock &BB) {
> > + if (!isa<ReturnInst>(BB.getTerminator()))
> > + return false;
> > + return none_of(BB, instructionDoesNotReturn);
> > +}
> > +
> > +// Set the noreturn function attribute if possible.
> > +static bool addNoReturnAttrs(const SCCNodeSet &SCCNodes) {
> > + bool Changed = false;
> > +
> > + for (Function *F : SCCNodes) {
> > + if (!F || !F->hasExactDefinition() ||
> F->hasFnAttribute(Attribute::Naked) ||
> > + F->doesNotReturn())
> > + continue;
> > +
> > + // The function can return if any basic blocks can return.
> > + // FIXME: this doesn't handle recursion or unreachable blocks.
> > + if (none_of(*F, basicBlockCanReturn)) {
> > + F->setDoesNotReturn();
> > + Changed = true;
> > + }
> > + }
> > +
> > + return Changed;
> > +}
> > +
> > static SCCNodesResult createSCCNodeSet(ArrayRef<Function *> Functions)
> {
> > SCCNodesResult Res;
> > Res.HasUnknownCall = false;
> > @@ -1431,6 +1467,7 @@ static bool
> deriveAttrsInPostOrder(ArrayRef<Function *> Functions,
> > Changed |= addReadAttrs(Nodes.SCCNodes, AARGetter);
> > Changed |= addArgumentAttrs(Nodes.SCCNodes);
> > Changed |= inferConvergent(Nodes.SCCNodes);
> > + Changed |= addNoReturnAttrs(Nodes.SCCNodes);
> >
> > // If we have no external nodes participating in the SCC, we can
> deduce some
> > // more precise attributes as well.
> >
> > diff --git a/llvm/test/Transforms/FunctionAttrs/noreturn.ll
> b/llvm/test/Transforms/FunctionAttrs/noreturn.ll
> > new file mode 100644
> > index 000000000000..acc538d3ccee
> > --- /dev/null
> > +++ b/llvm/test/Transforms/FunctionAttrs/noreturn.ll
> > @@ -0,0 +1,66 @@
> > +; RUN: opt < %s -passes='function-attrs' -S | FileCheck %s
> > +
> > +declare i32 @f()
> > +
> > +; CHECK: Function Attrs: noreturn
> > +; CHECK-NEXT: @noreturn()
> > +declare i32 @noreturn() noreturn
> > +
> > +; CHECK: Function Attrs: noreturn
> > +; CHECK-NEXT: @caller()
> > +define i32 @caller() {
> > + %c = call i32 @noreturn()
> > + ret i32 %c
> > +}
> > +
> > +; CHECK: Function Attrs: noreturn
> > +; CHECK-NEXT: @caller2()
> > +define i32 @caller2() {
> > + %c = call i32 @caller()
> > + ret i32 %c
> > +}
> > +
> > +; CHECK: Function Attrs: noreturn
> > +; CHECK-NEXT: @caller3()
> > +define i32 @caller3() {
> > +entry:
> > + br label %end
> > +end:
> > + %c = call i32 @noreturn()
> > + ret i32 %c
> > +}
> > +
> > +; CHECK-NOT: Function Attrs: {{.*}}noreturn
> > +; CHECK: define i32 @caller4()
> > +define i32 @caller4() {
> > +entry:
> > + br label %end
> > +end:
> > + %c = call i32 @f()
> > + ret i32 %c
> > +}
> > +
> > +; CHECK-NOT: Function Attrs: {{.*}}noreturn
> > +; CHECK: @caller5()
> > +; We currently don't handle unreachable blocks.
> > +define i32 @caller5() {
> > +entry:
> > + %c = call i32 @noreturn()
> > + ret i32 %c
> > +unreach:
> > + %d = call i32 @f()
> > + ret i32 %d
> > +}
> > +
> > +; CHECK-NOT: Function Attrs: {{.*}}noreturn
> > +; CHECK: @caller6()
> > +define i32 @caller6() naked {
> > + %c = call i32 @noreturn()
> > + ret i32 %c
> > +}
> > +
> > +; CHECK: Function Attrs: {{.*}}noreturn
> > +; CHECK-NEXT: @alreadynoreturn()
> > +define i32 @alreadynoreturn() noreturn {
> > + unreachable
> > +}
> >
> > diff --git a/llvm/test/Transforms/PruneEH/simplenoreturntest.ll
> b/llvm/test/Transforms/PruneEH/simplenoreturntest.ll
> > index 814f8b4a686f..ccb9f5eb6f59 100644
> > --- a/llvm/test/Transforms/PruneEH/simplenoreturntest.ll
> > +++ b/llvm/test/Transforms/PruneEH/simplenoreturntest.ll
> > @@ -1,4 +1,5 @@
> > ; RUN: opt < %s -prune-eh -S -enable-new-pm=0 | not grep "ret i32"
> > +; RUN: opt < %s -passes='function-attrs,function(simplifycfg)' -S | not
> grep "ret i32"
> >
> > declare void @noreturn() noreturn
> >
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at lists.llvm.org
> > https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210107/e5b21a3f/attachment.html>
More information about the llvm-commits
mailing list