Index: include/llvm/Analysis/AliasAnalysis.h =================================================================== --- include/llvm/Analysis/AliasAnalysis.h +++ include/llvm/Analysis/AliasAnalysis.h @@ -231,6 +231,14 @@ FMRB_OnlyReadsArgumentPointees = FMRL_ArgumentPointees | static_cast(ModRefInfo::Ref), + /// The only memory references in this function (if it has any) are + /// non-volatile stores to objects pointed to by its pointer-typed + /// arguments, with arbitrary offsets. + /// + /// This property corresponds to the IntrWriteArgMem LLVM intrinsic flag. + FMRB_OnlyWritesArgumentPointees = + FMRL_ArgumentPointees | static_cast(ModRefInfo::Mod), + /// The only memory references in this function (if it has any) are /// non-volatile loads and stores from objects pointed to by its /// pointer-typed arguments, with arbitrary offsets. Index: include/llvm/Transforms/IPO/FunctionAttrs.h =================================================================== --- include/llvm/Transforms/IPO/FunctionAttrs.h +++ include/llvm/Transforms/IPO/FunctionAttrs.h @@ -18,6 +18,7 @@ #include "llvm/Analysis/CGSCCPassManager.h" #include "llvm/Analysis/LazyCallGraph.h" +#include "llvm/Analysis/AliasAnalysis.h" #include "llvm/IR/PassManager.h" namespace llvm { @@ -27,17 +28,9 @@ class Module; class Pass; -/// The three kinds of memory access relevant to 'readonly' and -/// 'readnone' attributes. -enum MemoryAccessKind { - MAK_ReadNone = 0, - MAK_ReadOnly = 1, - MAK_MayWrite = 2, - MAK_WriteOnly = 3 -}; - /// Returns the memory access properties of this copy of the function. -MemoryAccessKind computeFunctionBodyMemoryAccess(Function &F, AAResults &AAR); +FunctionModRefBehavior computeFunctionBodyMemoryAccess(Function &F, + AAResults &AAR); /// Computes function attributes in post-order over the call graph. /// Index: lib/Transforms/IPO/FunctionAttrs.cpp =================================================================== --- lib/Transforms/IPO/FunctionAttrs.cpp +++ lib/Transforms/IPO/FunctionAttrs.cpp @@ -65,6 +65,7 @@ #define DEBUG_TYPE "functionattrs" STATISTIC(NumReadNone, "Number of functions marked readnone"); +STATISTIC(NumArgMemOnly, "Number of functions marked argmemonly"); STATISTIC(NumReadOnly, "Number of functions marked readonly"); STATISTIC(NumWriteOnly, "Number of functions marked writeonly"); STATISTIC(NumNoCapture, "Number of arguments marked nocapture"); @@ -94,7 +95,7 @@ } // end anonymous namespace -/// Returns the memory access attribute for function F using AAR for AA results, +/// Returns the mod/ref behavior for the function F using AAR for AA results, /// where SCCNodes is the current SCC. /// /// If ThisBody is true, this function may examine the function body and will @@ -102,31 +103,43 @@ /// result will be based only on AA results for the function declaration; it /// will be assumed that some other (perhaps less optimized) version of the /// function may be selected at link time. -static MemoryAccessKind checkFunctionMemoryAccess(Function &F, bool ThisBody, - AAResults &AAR, - const SCCNodeSet &SCCNodes) { +static FunctionModRefBehavior +checkFunctionMemoryAccess(Function &F, bool ThisBody, AAResults &AAR, + const SCCNodeSet &SCCNodes) { FunctionModRefBehavior MRB = AAR.getModRefBehavior(&F); - if (MRB == FMRB_DoesNotAccessMemory) - // Already perfect! - return MAK_ReadNone; - if (!ThisBody) { - if (AliasAnalysis::onlyReadsMemory(MRB)) - return MAK_ReadOnly; + // If we get readnone, or the body is not to be analyzed, we are done. + if (MRB == FMRB_DoesNotAccessMemory || !ThisBody) + return MRB; - if (AliasAnalysis::doesNotReadMemory(MRB)) - return MAK_WriteOnly; + // Initially we assume that we neither read nor write memory and that all + // accessed pointers are derived from arguments. + bool ArgumentsOnly = true; + bool ReadsMemory = false; + bool WritesMemory = false; - // Conservatively assume it reads and writes to memory. - return MAK_MayWrite; - } + // Helper function that checks if the object behind Ptr is an argument or a + // local value (alloca). Note that the function potentially modifies its + // argument to point to the underlying object. This is desirable as it can + // only improve the result of the pointsToConstantMemory calls that follow. + const DataLayout &DL = F.getParent()->getDataLayout(); + auto RefineAndCheckPtr = [&](Value *&Ptr) { + Ptr = GetUnderlyingObject(Ptr, DL); + return isa(Ptr) || isa(Ptr); + }; // Scan the function body for instructions that may read or write memory. - bool ReadsMemory = false; - bool WritesMemory = false; for (inst_iterator II = inst_begin(F), E = inst_end(F); II != E; ++II) { + // If we reached the worst possible outcome we can immediately stop. + if (ReadsMemory && WritesMemory && !ArgumentsOnly) + return FMRB_UnknownModRefBehavior; + Instruction *I = &*II; + // Flag to indicate we already checked if the instruction I will access + // only arguments and local memory. + bool ArgumentsOnlyCheckedForI = false; + // Some instructions can be ignored even if they read or write memory. // Detect these now, skipping to the next instruction if one is found. CallSite CS(cast(I)); @@ -146,6 +159,10 @@ continue; if (!AliasAnalysis::onlyAccessesArgPointees(MRB)) { + // If a call site does potentially access memory locations not derived + // from argument pointers so does the surrounding function. + ArgumentsOnly = false; + // The call could access any memory. If that includes writes, note it. if (isModSet(MRI)) WritesMemory = true; @@ -163,6 +180,12 @@ if (!Arg->getType()->isPtrOrPtrVectorTy()) continue; + // If we still have not accessed anything but function arguments, we + // check if the call argument is also a function argument or a local + // value (alloca). Also see the definition of RefineAndCheckPtr above. + if (ArgumentsOnly) + ArgumentsOnly = RefineAndCheckPtr(Arg); + AAMDNodes AAInfo; I->getAAMetadata(AAInfo); MemoryLocation Loc(Arg, MemoryLocation::UnknownSize, AAInfo); @@ -181,6 +204,13 @@ } continue; } else if (LoadInst *LI = dyn_cast(I)) { + // Check if the pointer operand is an argument or local value. + if (ArgumentsOnly) { + Value *Ptr = LI->getPointerOperand(); + ArgumentsOnly = RefineAndCheckPtr(Ptr); + ArgumentsOnlyCheckedForI = true; + } + // Ignore non-volatile loads from local memory. (Atomic is okay here.) if (!LI->isVolatile()) { MemoryLocation Loc = MemoryLocation::get(LI); @@ -188,6 +218,13 @@ continue; } } else if (StoreInst *SI = dyn_cast(I)) { + // Check if the pointer operand is an argument or local value. + if (ArgumentsOnly) { + Value *Ptr = SI->getPointerOperand(); + ArgumentsOnly = RefineAndCheckPtr(Ptr); + ArgumentsOnlyCheckedForI = true; + } + // Ignore non-volatile stores to local memory. (Atomic is okay here.) if (!SI->isVolatile()) { MemoryLocation Loc = MemoryLocation::get(SI); @@ -195,44 +232,75 @@ continue; } } else if (VAArgInst *VI = dyn_cast(I)) { + // Check if the pointer operand is an argument or local value. + if (ArgumentsOnly) { + Value *Ptr = VI->getPointerOperand(); + ArgumentsOnly = RefineAndCheckPtr(Ptr); + ArgumentsOnlyCheckedForI = true; + } + // Ignore vaargs on local memory. MemoryLocation Loc = MemoryLocation::get(VI); if (AAR.pointsToConstantMemory(Loc, /*OrLocal=*/true)) continue; } - // Any remaining instructions need to be taken seriously! Check if they - // read or write memory. - if (I->mayWriteToMemory()) - // Writes memory, remember that. + // Any remaining instructions need to be taken seriously! Check if they + // read or write memory and if so also drop the arguments only state. + // However, if we reached this point for a load, store, or vaarginst we + // have already checked the argument only state. + if (I->mayWriteToMemory()) { WritesMemory = true; - - // If this instruction may read memory, remember that. - ReadsMemory |= I->mayReadFromMemory(); + ArgumentsOnly = ArgumentsOnlyCheckedForI ? ArgumentsOnly : false; + } + if (I->mayReadFromMemory()) { + ReadsMemory = true; + ArgumentsOnly = ArgumentsOnlyCheckedForI ? ArgumentsOnly : false; + } } - if (WritesMemory) { - if (!ReadsMemory) - return MAK_WriteOnly; + // Check (almost) all combinations of reads/writes and arguments only to + // determine which function mod/ref behavior is appropriate. + if (!ReadsMemory && !WritesMemory) + return FMRB_DoesNotAccessMemory; + + if (ArgumentsOnly) { + if (ReadsMemory && WritesMemory) + return FMRB_OnlyAccessesArgumentPointees; + else if (ReadsMemory) + return FMRB_OnlyReadsArgumentPointees; + else + return FMRB_OnlyWritesArgumentPointees; + } else { + if (ReadsMemory && WritesMemory) + return FMRB_UnknownModRefBehavior; + else if (ReadsMemory) + return FMRB_OnlyReadsMemory; else - return MAK_MayWrite; + return FMRB_DoesNotReadMemory; } - - return ReadsMemory ? MAK_ReadOnly : MAK_ReadNone; } -MemoryAccessKind llvm::computeFunctionBodyMemoryAccess(Function &F, - AAResults &AAR) { +FunctionModRefBehavior llvm::computeFunctionBodyMemoryAccess(Function &F, + AAResults &AAR) { return checkFunctionMemoryAccess(F, /*ThisBody=*/true, AAR, {}); } -/// Deduce readonly/readnone attributes for the SCC. +/// Deduce readnone/readonly/writeonly and argmemonly attributes for the SCC. template -static bool addReadAttrs(const SCCNodeSet &SCCNodes, AARGetterT &&AARGetter) { - // Check if any of the functions in the SCC read or write memory. If they - // write memory then they can't be marked readnone or readonly. +static bool addMemoryAccessAttrs(const SCCNodeSet &SCCNodes, + AARGetterT &&AARGetter) { + // Check if all functions in the SCC + // - do not access memory, + // - only read memory, + // - only write memory. + // We also determine if all accessed memory locations are based on function + // arguments. If that is the case, we can employ the argmemonly attribute in + // addition to a __potential__ readnone/readonly/writeonly attribute. + bool ArgumentsOnly = true; bool ReadsMemory = false; bool WritesMemory = false; + for (Function *F : SCCNodes) { // Call the callable parameter to look up AA results for this function. AAResults &AAR = AARGetter(*F); @@ -242,57 +310,94 @@ // comment on GlobalValue::isDefinitionExact for more details. switch (checkFunctionMemoryAccess(*F, F->hasExactDefinition(), AAR, SCCNodes)) { - case MAK_MayWrite: - return false; - case MAK_ReadOnly: + case FMRB_OnlyReadsMemory: + ArgumentsOnly = false; + LLVM_FALLTHROUGH; + case FMRB_OnlyReadsArgumentPointees: ReadsMemory = true; break; - case MAK_WriteOnly: + case FMRB_DoesNotReadMemory: + ArgumentsOnly = false; + LLVM_FALLTHROUGH; + case FMRB_OnlyWritesArgumentPointees: WritesMemory = true; break; - case MAK_ReadNone: + case FMRB_DoesNotAccessMemory: // Nothing to do! break; + case FMRB_OnlyAccessesArgumentPointees: + ReadsMemory = WritesMemory = true; + break; + default: + return false; } } - // Success! Functions in this SCC do not access memory, or only read memory. - // Give them the appropriate attribute. + // Check if we could derive attributes and if so add them to the functions. + if (ReadsMemory && WritesMemory && !ArgumentsOnly) + return false; + bool MadeChange = false; - assert(!(ReadsMemory && WritesMemory) && - "Function marked read-only and write-only"); for (Function *F : SCCNodes) { if (F->doesNotAccessMemory()) // Already perfect! continue; - if (F->onlyReadsMemory() && ReadsMemory) - // No change. + // Set the argmemonly attribute if appropriate. + if (ArgumentsOnly && !F->onlyAccessesArgMemory()) { + F->addFnAttr(Attribute::ArgMemOnly); + ++NumArgMemOnly; + MadeChange = true; + } + + // If we only derived argmemonly skip the rest. + if (ReadsMemory && WritesMemory) continue; - if (F->doesNotReadMemory() && WritesMemory) + if (F->onlyReadsMemory()) { + // Check if we can refine readonly to readnone. + if (!ReadsMemory) { + F->removeFnAttr(Attribute::ReadOnly); + F->removeFnAttr(Attribute::ArgMemOnly); + F->addFnAttr(Attribute::ReadNone); + MadeChange = true; + } continue; + } - MadeChange = true; + if (F->doesNotReadMemory()) { + // Check if we can refine writeonly to readnone. + if (!WritesMemory) { + F->removeFnAttr(Attribute::WriteOnly); + F->removeFnAttr(Attribute::ArgMemOnly); + F->addFnAttr(Attribute::ReadNone); + MadeChange = true; + } + continue; + } + + // If there were existing attributes, we should have handled them above. + assert(!F->hasFnAttribute(Attribute::ReadOnly) && + !F->hasFnAttribute(Attribute::ReadNone) && + !F->hasFnAttribute(Attribute::WriteOnly)); - // Clear out any existing attributes. - F->removeFnAttr(Attribute::ReadOnly); - F->removeFnAttr(Attribute::ReadNone); - F->removeFnAttr(Attribute::WriteOnly); + MadeChange = true; // Add in the new attribute. - if(WritesMemory && !ReadsMemory) - F->addFnAttr(Attribute::WriteOnly); - else - F->addFnAttr(ReadsMemory ? Attribute::ReadOnly : Attribute::ReadNone); + assert(!ReadsMemory || !WritesMemory); - if (WritesMemory && !ReadsMemory) + if (WritesMemory) { + F->addFnAttr(Attribute::WriteOnly); ++NumWriteOnly; - else if (ReadsMemory) + } else if (ReadsMemory) { + F->addFnAttr(Attribute::ReadOnly); ++NumReadOnly; - else + } else { + F->removeFnAttr(Attribute::ArgMemOnly); + F->addFnAttr(Attribute::ReadNone); ++NumReadNone; + } } return MadeChange; @@ -1358,7 +1463,7 @@ bool Changed = false; Changed |= addArgumentReturnedAttrs(SCCNodes); - Changed |= addReadAttrs(SCCNodes, AARGetter); + Changed |= addMemoryAccessAttrs(SCCNodes, AARGetter); Changed |= addArgumentAttrs(SCCNodes); // If we have no external nodes participating in the SCC, we can deduce some @@ -1436,7 +1541,7 @@ return Changed; Changed |= addArgumentReturnedAttrs(SCCNodes); - Changed |= addReadAttrs(SCCNodes, AARGetter); + Changed |= addMemoryAccessAttrs(SCCNodes, AARGetter); Changed |= addArgumentAttrs(SCCNodes); // If we have no external nodes participating in the SCC, we can deduce some Index: lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp =================================================================== --- lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp +++ lib/Transforms/IPO/ThinLTOBitcodeWriter.cpp @@ -256,7 +256,8 @@ return; } if (!F->isDeclaration() && - computeFunctionBodyMemoryAccess(*F, AARGetter(*F)) == MAK_ReadNone) + computeFunctionBodyMemoryAccess(*F, AARGetter(*F)) == + FMRB_DoesNotAccessMemory) EligibleVirtualFns.insert(F); }); } Index: lib/Transforms/IPO/WholeProgramDevirt.cpp =================================================================== --- lib/Transforms/IPO/WholeProgramDevirt.cpp +++ lib/Transforms/IPO/WholeProgramDevirt.cpp @@ -1200,7 +1200,7 @@ for (VirtualCallTarget &Target : TargetsForSlot) { if (Target.Fn->isDeclaration() || computeFunctionBodyMemoryAccess(*Target.Fn, AARGetter(*Target.Fn)) != - MAK_ReadNone || + FMRB_DoesNotAccessMemory || Target.Fn->arg_empty() || !Target.Fn->arg_begin()->use_empty() || Target.Fn->getReturnType() != RetType) return false; Index: test/Analysis/TypeBasedAliasAnalysis/argmemonly.ll =================================================================== --- /dev/null +++ test/Analysis/TypeBasedAliasAnalysis/argmemonly.ll @@ -0,0 +1,64 @@ +; RUN: opt < %s -functionattrs -S | FileCheck %s +; +target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128" + +; CHECK: Function Attrs: argmemonly noinline nounwind uwtable +; CHECK: define dso_local void @read_write_arg(i32* nocapture %W, i32* nocapture readonly %R) #0 +; +; Function Attrs: noinline nounwind uwtable +define dso_local void @read_write_arg(i32* %W, i32* %R) #0 { +entry: + %call = call i32 @readonlyarg(i32* %R) + call void @writeonlyarg(i32* %W, i32 %call) + ret void +} + +; CHECK: Function Attrs: argmemonly noinline nounwind uwtable writeonly +; CHECK: define dso_local void @writeonlyarg(i32* nocapture %W, i32 %v) #1 +; +; Function Attrs: noinline nounwind uwtable +define dso_local void @writeonlyarg(i32* %W, i32 %v) #0 { +entry: + %tobool = icmp eq i32 %v, 0 + br i1 %tobool, label %if.end, label %if.then + +if.then: ; preds = %entry + %add.ptr = getelementptr inbounds i32, i32* %W, i64 1 + %sub = add nsw i32 %v, -1 + call void @writeonlyarg(i32* nonnull %add.ptr, i32 %sub) + br label %if.end + +if.end: ; preds = %entry, %if.then + store i32 %v, i32* %W, align 4 + ret void +} + +; CHECK: Function Attrs: argmemonly noinline nounwind readonly uwtable +; CHECK: define dso_local i32 @readonlyarg(i32* nocapture readonly %R) #2 +; +; Function Attrs: noinline nounwind uwtable +define dso_local i32 @readonlyarg(i32* %R) #0 { +entry: + %0 = load i32, i32* %R, align 4 + %tobool = icmp eq i32 %0, 0 + br i1 %tobool, label %if.end, label %if.then + +if.then: ; preds = %entry + %1 = load i32, i32* %R, align 4 + br label %return + +if.end: ; preds = %entry + %add.ptr = getelementptr inbounds i32, i32* %R, i64 1 + %call = call i32 @readonlyarg(i32* nonnull %add.ptr) + br label %return + +return: ; preds = %if.end, %if.then + %retval.0 = phi i32 [ %1, %if.then ], [ %call, %if.end ] + ret i32 %retval.0 +} + +attributes #0 = { noinline nounwind uwtable } + +; CHECK: attributes #0 = { argmemonly noinline nounwind uwtable } +; CHECK: attributes #1 = { argmemonly noinline nounwind uwtable writeonly } +; CHECK: attributes #2 = { argmemonly noinline nounwind readonly uwtable } Index: test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll =================================================================== --- test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll +++ test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll @@ -49,7 +49,7 @@ ret void } -; CHECK: define void @test2_no(i8* nocapture %p, i8* nocapture readonly %q, i64 %n) #3 { +; CHECK: define void @test2_no(i8* nocapture %p, i8* nocapture readonly %q, i64 %n) #5 { define void @test2_no(i8* %p, i8* %q, i64 %n) nounwind { call void @llvm.memcpy.p0i8.p0i8.i64(i8* %p, i8* %q, i64 %n, i1 false), !tbaa !2 ret void @@ -63,7 +63,7 @@ ret i32 %t } -; CHECK: define i32 @test3_no(i8* nocapture %p) #5 { +; CHECK: define i32 @test3_no(i8* nocapture %p) #6 { define i32 @test3_no(i8* %p) nounwind { %t = va_arg i8* %p, i32, !tbaa !2 ret i32 %t @@ -73,12 +73,12 @@ declare void @llvm.memcpy.p0i8.p0i8.i64(i8*, i8*, i64, i1) nounwind ; CHECK: attributes #0 = { norecurse nounwind readnone } -; CHECK: attributes #1 = { norecurse nounwind writeonly } +; CHECK: attributes #1 = { argmemonly norecurse nounwind writeonly } ; CHECK: attributes #2 = { nounwind readonly } ; CHECK: attributes #3 = { nounwind } ; CHECK: attributes #4 = { nounwind readnone } -; CHECK: attributes #5 = { norecurse nounwind } -; CHECK: attributes #6 = { argmemonly nounwind } +; CHECK: attributes #5 = { argmemonly nounwind } +; CHECK: attributes #6 = { argmemonly norecurse nounwind } ; Root note. !0 = !{ } Index: test/Transforms/FunctionAttrs/atomic.ll =================================================================== --- test/Transforms/FunctionAttrs/atomic.ll +++ test/Transforms/FunctionAttrs/atomic.ll @@ -21,4 +21,4 @@ } ; CHECK: attributes #0 = { norecurse nounwind readnone ssp uwtable } -; CHECK: attributes #1 = { norecurse nounwind ssp uwtable } +; CHECK: attributes #1 = { argmemonly norecurse nounwind ssp uwtable }