From 5246e0b2c25c293e6896e3a6fff63ade00fcb54e Mon Sep 17 00:00:00 2001 From: David Majnemer Date: Tue, 19 Jul 2016 18:50:26 +0000 Subject: [PATCH] [FunctionAttrs] Correct the safety analysis for inference of 'returned' We skipped over ReturnInsts which didn't return an argument which would lead us to incorrectly conclude that an argument returned by another ReturnInst was 'returned'. This reverts commit r275756. This fixes PR28610. llvm-svn: 276008 --- llvm/lib/Transforms/IPO/FunctionAttrs.cpp | 51 ++++++++++++++++++++++ .../FunctionAttrs/2009-01-02-LocalStores.ll | 2 +- llvm/test/Transforms/FunctionAttrs/nocapture.ll | 6 +-- llvm/test/Transforms/FunctionAttrs/readattrs.ll | 4 +- 4 files changed, 57 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp index fff5440..b5d04a7 100644 --- a/llvm/lib/Transforms/IPO/FunctionAttrs.cpp +++ b/llvm/lib/Transforms/IPO/FunctionAttrs.cpp @@ -42,6 +42,7 @@ using namespace llvm; STATISTIC(NumReadNone, "Number of functions marked readnone"); STATISTIC(NumReadOnly, "Number of functions marked readonly"); STATISTIC(NumNoCapture, "Number of arguments marked nocapture"); +STATISTIC(NumReturned, "Number of arguments marked returned"); STATISTIC(NumReadNoneArg, "Number of arguments marked readnone"); STATISTIC(NumReadOnlyArg, "Number of arguments marked readonly"); STATISTIC(NumNoAlias, "Number of function returns marked noalias"); @@ -483,6 +484,54 @@ determinePointerReadAttrs(Argument *A, return IsRead ? Attribute::ReadOnly : Attribute::ReadNone; } +/// Deduce returned attributes for the SCC. +static bool addArgumentReturnedAttrs(const SCCNodeSet &SCCNodes) { + bool Changed = false; + + AttrBuilder B; + B.addAttribute(Attribute::Returned); + + // Check each function in turn, determining if an argument is always returned. + for (Function *F : SCCNodes) { + // We can infer and propagate function attributes only when we know that the + // definition we'll get at link time is *exactly* the definition we see now. + // For more details, see GlobalValue::mayBeDerefined. + if (!F->hasExactDefinition()) + continue; + + if (F->getReturnType()->isVoidTy()) + continue; + + auto FindRetArg = [&]() -> Value * { + Value *RetArg = nullptr; + for (BasicBlock &BB : *F) + if (auto *Ret = dyn_cast(BB.getTerminator())) { + // Note that stripPointerCasts should look through functions with + // returned arguments. + Value *RetVal = Ret->getReturnValue()->stripPointerCasts(); + if (!isa(RetVal) || RetVal->getType() != F->getReturnType()) + return nullptr; + + if (!RetArg) + RetArg = RetVal; + else if (RetArg != RetVal) + return nullptr; + } + + return RetArg; + }; + + if (Value *RetArg = FindRetArg()) { + auto *A = cast(RetArg); + A->addAttr(AttributeSet::get(F->getContext(), A->getArgNo() + 1, B)); + ++NumReturned; + Changed = true; + } + } + + return Changed; +} + /// Deduce nocapture attributes for the SCC. static bool addArgumentAttrs(const SCCNodeSet &SCCNodes) { bool Changed = false; @@ -1024,6 +1073,7 @@ PreservedAnalyses PostOrderFunctionAttrsPass::run(LazyCallGraph::SCC &C, } bool Changed = false; + Changed |= addArgumentReturnedAttrs(SCCNodes); Changed |= addReadAttrs(SCCNodes, AARGetter); Changed |= addArgumentAttrs(SCCNodes); @@ -1089,6 +1139,7 @@ static bool runImpl(CallGraphSCC &SCC, AARGetterT AARGetter) { SCCNodes.insert(F); } + Changed |= addArgumentReturnedAttrs(SCCNodes); Changed |= addReadAttrs(SCCNodes, AARGetter); Changed |= addArgumentAttrs(SCCNodes); diff --git a/llvm/test/Transforms/FunctionAttrs/2009-01-02-LocalStores.ll b/llvm/test/Transforms/FunctionAttrs/2009-01-02-LocalStores.ll index ec1db09..0d0231b 100644 --- a/llvm/test/Transforms/FunctionAttrs/2009-01-02-LocalStores.ll +++ b/llvm/test/Transforms/FunctionAttrs/2009-01-02-LocalStores.ll @@ -14,7 +14,7 @@ define i32* @b(i32 *%q) { ret i32* %tmp } -; CHECK: define i32* @c(i32* readnone %r) +; CHECK: define i32* @c(i32* readnone returned %r) @g = global i32 0 define i32* @c(i32 *%r) { %a = icmp eq i32* %r, null diff --git a/llvm/test/Transforms/FunctionAttrs/nocapture.ll b/llvm/test/Transforms/FunctionAttrs/nocapture.ll index 020d8bc..e137a38 100644 --- a/llvm/test/Transforms/FunctionAttrs/nocapture.ll +++ b/llvm/test/Transforms/FunctionAttrs/nocapture.ll @@ -1,7 +1,7 @@ ; RUN: opt < %s -functionattrs -S | FileCheck %s @g = global i32* null ; [#uses=1] -; CHECK: define i32* @c1(i32* readnone %q) +; CHECK: define i32* @c1(i32* readnone returned %q) define i32* @c1(i32* %q) { ret i32* %q } @@ -140,7 +140,7 @@ define void @test1_1(i8* %x1_1, i8* %y1_1) { ret void } -; CHECK: define i8* @test1_2(i8* nocapture readnone %x1_2, i8* %y1_2) +; CHECK: define i8* @test1_2(i8* nocapture readnone %x1_2, i8* returned %y1_2) define i8* @test1_2(i8* %x1_2, i8* %y1_2) { call void @test1_1(i8* %x1_2, i8* %y1_2) store i32* null, i32** @g @@ -168,7 +168,7 @@ define void @test4_1(i8* %x4_1) { ret void } -; CHECK: define i8* @test4_2(i8* nocapture readnone %x4_2, i8* readnone %y4_2, i8* nocapture readnone %z4_2) +; CHECK: define i8* @test4_2(i8* nocapture readnone %x4_2, i8* readnone returned %y4_2, i8* nocapture readnone %z4_2) define i8* @test4_2(i8* %x4_2, i8* %y4_2, i8* %z4_2) { call void @test4_1(i8* null) store i32* null, i32** @g diff --git a/llvm/test/Transforms/FunctionAttrs/readattrs.ll b/llvm/test/Transforms/FunctionAttrs/readattrs.ll index 65861a5..988557e2 100644 --- a/llvm/test/Transforms/FunctionAttrs/readattrs.ll +++ b/llvm/test/Transforms/FunctionAttrs/readattrs.ll @@ -11,7 +11,7 @@ define void @test1_2(i8* %x1_2, i8* %y1_2, i8* %z1_2) { ret void } -; CHECK: define i8* @test2(i8* readnone %p) +; CHECK: define i8* @test2(i8* readnone returned %p) define i8* @test2(i8* %p) { store i32 0, i32* @x ret i8* %p @@ -53,7 +53,7 @@ define void @test7_1(i32* inalloca %a) { ret void } -; CHECK: define i32* @test8_1(i32* readnone %p) +; CHECK: define i32* @test8_1(i32* readnone returned %p) define i32* @test8_1(i32* %p) { entry: ret i32* %p -- 2.7.4