[Attributor] Deduce attributes for non-exact functions

[jdoerfert]

Summary

In the LLVM test suite and SPEC2006, ~67% of all function declarations
are non-exact, thus they can be replaced at link-time. While we cannot
generally use them to perform inter-procedural (IP) reasoning, it is
allowed to "internalize" them first and derive IP information
afterwards. In fact, we can use the information we derived assuming they
have exact definitions to reason if internalization might be beneficial.

This patch allows the Attributor to employ shallow wrappers, the
cheapest internalization method I could think of. A shallow wrapper is a
function with the same type (and attributes) as the original one. Inside
the wrapper there is only call to the original one and a return of the
result. The scheme is shown below:

Assuming the declaration of looks like:

rty F(aty0 arg0, ..., atyN argN);

The wrapper will then look as follows:

rty wrapper(aty0 arg0, ..., atyN argN) {
  return F(arg0, ..., argN);
}

Once the wrapper was created we can change the linkage type of the
original function to internal which allows us to use it for recursive
reasoning. We can also manifest the results, e.g., annotate the
arguments with attributes.

Shallow wrappers are cheap because the new internal function has a
single call site. This likely means we inline them later which results
in "similar" code as we started out with except that passes in-between
the Attributor and the inliner can use the annotated information.

A separate patch will introduce deep wrappers that replace the known
uses of the function with the internalized version. Afterwards, we need
to work on a cost heuristic. I will also see if we can disable inlining
for "wrapper-like" functions and post my findings on the list.

[nicholas]

We can, however, use information from non-exact definitions to improve
themselves as they are either replaced as a whole or not at all.

I didn't understand the motivation of this patch by reading the description, I understood it with this comment. In particular I had the impression that you were using the attributes of one internalized function to optimize another internalized function.

---

I'm curious what function-local optimizations we're missing? If I understand the design, you can deduce that a function is 'readnone', but no callers of the function can be optimized based on that deduction, since the 'readnone' body may be replaced. I'm mostly confident we don't use readnone to optimize the body of the function. Do you have an example of an attribute that we can deduce and then use to optimize the body of the function?

---

If it were my design de novo, I would prefer treating the function attributes as being "as authoritative as the linkage", which pushes the problem of deciding whether F->doesNotReturn() is sufficient to the caller, it needs to check the linkage (or isExactDefinition) before asking that question. Unfortunately that would churn the API as we would need to update attribute consumers to use a convenience wrapper API when they're examining a callee instead of their function parent, but I expect pushing that decision into the pass might be necessary to get corner cases optimal and correct in passes that look at multiple functions. That seems to me like it would be a lot of work, what do you think?

Even if the wrapper function approach works, I don't think the representation is right. There aren't two functions, which is why you'll need to teach the inliner, etc., that these aren't truly functions. Without adding the concept to the LangRef, you've got two separate passes in LLVM that coordinate optimizations with each other though implementation details.

I'm not sure the wrapper can be used on functions that aren't marked unnamed_addr or you prove the address isn't taken? You update all users, which means that someone who takes the address of the function in this module might get a different address than someone who takes the address in a different module since they're two different internal functions now? Those pointers need to compare equal if you pass the function pointer around.

llvm/lib/Transforms/IPO/Attributor.cpp

1187

FYI, I don't think we have a reserved prefix for llvm-produced functions, but you can make it anonymous with F.setName("");. Since the function has internal linkage, it doesn't need a name to link with other modules.

[jdoerfert]

Thanks for the quick feedback!

In D63312#1542977, @nicholas wrote:

We can, however, use information from non-exact definitions to improve
themselves as they are either replaced as a whole or not at all.

I didn't understand the motivation of this patch by reading the description, I understood it with this comment. In particular I had the impression that you were using the attributes of one internalized function to optimize another internalized function.

With this patch there is no "inter-internalized-function communication". I have another one to do that. I think I have to reword the whole commit message to make it clear.

With these shallow wrappers you can only use the function itself when it comes to inter-procedural (IP) reasoning. That is why I need to check WrappedFunctions in mayDependOnNonExactDefinition.
You cannot do cross function reasoning because we actually do call the shallow wrapper which is a non-exact decleration. However, you can do recursive reasoning and manifest attributes on the function, parameters and return value of the internalized version.

I'm curious what function-local optimizations we're missing? If I understand the design, you can deduce that a function is 'readnone', but no callers of the function can be optimized based on that deduction, since the 'readnone' body may be replaced. I'm mostly confident we don't use readnone to optimize the body of the function. Do you have an example of an attribute that we can deduce and then use to optimize the body of the function?

The shallow wrappers are not the solution I want to have. Real internalization is what we actually want (later patch).
Shallow wrappers are however not useless. We could, for example, derive nofree and nosync on them which together allow to transform dereferenceable arguments into dereferenceable_globally (D61652) arguments. Invariant loads of the latter can be hoisted out of loops. Direct recursion can be optimized this way as well.

I did run a test on the LLVM-TS and SPEC2006 with "returned" and "nocapture" attribute enabled in the Attributor.
The changes in statistics > 1% are listed here: https://gist.github.com/jdoerfert/aa94861fd59d1564436d2ab490164d19
There was <5 benchmarks that were impacted wrt. compile and runtime. I don't have the numbers right now but I will show more elaborate results before we turn anything on. With deep wrappers 7 or so benchmarks got actually faster, 3-30% if I remember correctly.

If it were my design de novo, I would prefer treating the function attributes as being "as authoritative as the linkage", which pushes the problem of deciding whether F->doesNotReturn() is sufficient to the caller, it needs to check the linkage (or isExactDefinition) before asking that question. Unfortunately that would churn the API as we would need to update attribute consumers to use a convenience wrapper API when they're examining a callee instead of their function parent, but I expect pushing that decision into the pass might be necessary to get corner cases optimal and correct in passes that look at multiple functions. That seems to me like it would be a lot of work, what do you think?

I dislike this solution mostly because it makes user information less valuable (assuming I understand you correctly). If the user gives us information, __restrict__ -> noalias, int & -> dereferenceable(sizeof(int)), etc. we should be able to use that to the fullest extend regardless of the linkage.

Even if the wrapper function approach works, I don't think the representation is right. There aren't two functions, which is why you'll need to teach the inliner, etc., that these aren't truly functions. Without adding the concept to the LangRef, you've got two separate passes in LLVM that coordinate optimizations with each other though implementation details.

Even without teaching the inliner anything, deep wrappers did show a performance benefit (see above). I actually want to teach the inliner something, but I was hoping it is something general, namely:
If a function only contains a call and a return and the call can be implemented with a single jmp instruction (the arguments are passed in order,...), do not inline the call. This seems to me like a heuristic that could even save compile time while not sacrificing performance. I hope it would be applicable to user code as well. Even if not, I actually have another patch to use a similar wrapper concept in conjuction with the __attribute__((callback(...))). The use case there is to allow us to unpack struct arguments with more than 3 members without ever actually increasing/changing the arguments of a function. That is useful to communicate information about the struct members to the callee (and potentially back).

I'm not sure the wrapper can be used on functions that aren't marked unnamed_addr or you prove the address isn't taken? You update all users, which means that someone who takes the address of the function in this module might get a different address than someone who takes the address in a different module since they're two different internal functions now? Those pointers need to compare equal if you pass the function pointer around.

That is a very good point which I haven't considered yet. I will check for unnamed_addr and, if not present, check that the address is not taken.

llvm/lib/Transforms/IPO/Attributor.cpp

1187

I'll do that. The Prefix was just to avoid clashes ("__" is often reserved).

1310

This lambda got somehow in this review, will be committed before.

[jdoerfert]

Became D76404 which has landed.

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

private:
 
   /// A map from abstract attributes to the ones that queried them through calls
   /// to the getAAFor<...>(...) method.
   ///{
   using QueryMapTy =
       DenseMap<AbstractAttribute *, SetVector<AbstractAttribute *>>;
   QueryMapTy QueryMap;
   ///}
+
+  /// Check if the state of the abstract attribute \p AA may depend on
+  /// information derived from a non-exact definition.
+  ///
+  /// We cannot use information from non-exact definitions to improve other
+  /// definitions as the non-exact ones might be redefined at link-time which
+  /// could invalidate the result.
+  ///
+  /// We can, however, use information from non-exact definitions to improve
+  /// themselves as they are either replaced as a whole or not at all.
+  ///
+  /// \param AA The abstract attribute checked.
+  /// \param WrappedFunctions Functions now enclosed in a shallow wrapper.
+  /// \param DepMap A map from abstract attributes to others they depend on.
+  /// \param Cache A cache with known results.
+  bool mayDependOnNonExactDefinition(
+      AbstractAttribute &AA, SmallPtrSetImpl<Function *> &WrappedFunctions,
+      QueryMapTy &DepMap, DenseMap<AbstractAttribute *, Optional<bool>> &Cache);
 };
 
 /// Data structure to hold cached (LLVM-IR) information.
 ///
 /// All attributes are given an InformationCache object at creation time to
 /// avoid inspection of the IR by all of them individually. This default
 /// InformationCache will hold information required by 'default' attributes,
 /// thus the ones deduced when Attributor::identifyDefaultAbstractAttributes(..)

llvm/lib/Transforms/IPO/Attributor.cpp

 using namespace llvm;
 
 #define DEBUG_TYPE "attributor"
 
 STATISTIC(NumFnWithExactDefinition,
           "Number of function with exact definitions");
 STATISTIC(NumFnWithoutExactDefinition,
           "Number of function without exact definitions");
+STATISTIC(NumFnShallowWrapperCreated, "Number of shallow wrappers created");
+STATISTIC(NumAttrsRequiredDeepWrapper,
+          "Number of non-trivial attributes requiring a deep wrapper");
 STATISTIC(NumAttributesTimedOut,
           "Number of abstract attributes timed out before fixpoint");
 STATISTIC(NumAttributesValidFixpoint,
           "Number of abstract attributes in a valid fixpoint state");
 STATISTIC(NumAttributesManifested,
           "Number of abstract attributes manifested in IR");
 
 STATISTIC(NumFnUniqueReturned, "Number of function with unique return");
static cl::opt<bool> DisableAttributor(
     cl::init(false));
 
 static cl::opt<bool> VerifyAttributor(
     "attributor-verify", cl::Hidden,
     cl::desc("Verify the Attributor deduction and "
              "manifestation of attributes -- may issue false-positive errors"),
     cl::init(false));
 
+static cl::opt<bool>
+    AllowShallowWrappers("attributor-allow-shallow-wrappers", cl::Hidden,
+                         cl::desc("Allow the Attributor to create shallow "
+                                  "wrappers for non-exact definitions."),
+                         cl::init(false));
+
 /// Logic operators for the change status enum class.
 ///
 ///{
 ChangeStatus llvm::operator|(ChangeStatus l, ChangeStatus r) {
   return l == ChangeStatus::CHANGED ? l : r;
 }
 ChangeStatus llvm::operator&(ChangeStatus l, ChangeStatus r) {
   return l == ChangeStatus::UNCHANGED ? l : r;
if (Argument *AssumedRetArg = dyn_cast<Argument>(AssumedRetVal))
                                RetCS.getArgOperand(AssumedRetArg->getArgNo()),
                                ReturnInsts, AddRVs);
     else
       AddRVs[AssumedRetVal].insert(ReturnInsts.begin(), ReturnInsts.end());
   }
 
   ChangeStatus Changed = ChangeStatus::UNCHANGED;
 
+  // Update the return values set after we stopped iterating over it.
   for (auto &It : AddRVs) {
     assert(!It.second.empty() && "Entry does not add anything.");
     auto &ReturnInsts = ReturnedValues[It.first];
     for (ReturnInst *RI : It.second)
       if (ReturnInsts.insert(RI).second) {
         LLVM_DEBUG(dbgs() << "[AAReturnedValues] Add new returned value "
                           << *It.first << " => " << *RI << "\n");
         Changed = ChangeStatus::CHANGED;
ChangeStatus Attributor::run() {
   }
 
   LLVM_DEBUG({
     if (!Visited.empty())
       dbgs() << "\n[Attributor] Finalized " << Visited.size()
              << " abstract attributes.\n";
   });
 
+  SmallPtrSet<Function *, 32> WrappedFunctions;
+  DenseMap<AbstractAttribute *, Optional<bool>> NonExactDefinitionCache;
+  QueryMapTy DepMap;
+  for (auto &It : QueryMap)
+    for (AbstractAttribute *QuerriedAA : It.second)
+      DepMap[QuerriedAA].insert(It.first);
+
   unsigned NumManifested = 0;
   unsigned NumAtFixpoint = 0;
   ChangeStatus ManifestChange = ChangeStatus::UNCHANGED;
   for (AbstractAttribute *AA : AllAbstractAttributes) {
     AbstractState &State = AA->getState();
 
     // If there is not already a fixpoint reached, we can now take the
     // optimistic state. This is correct because we enforced a pessimistic one
     // on abstract attributes that were transitively dependent on a changed one
     // already above.
     if (!State.isAtFixpoint())
       State.indicateOptimisticFixpoint();
 
     // If the state is invalid, we do not try to manifest it.
     if (!State.isValidState())
       continue;
 
+    // We cannot manifest the state if it maybe dependent on non-exact
+    // definitions that could be replaced at link-time.
+    if (mayDependOnNonExactDefinition(*AA, WrappedFunctions, DepMap,
+                                      NonExactDefinitionCache))
+      continue;
+
     // Manifest the state and record if we changed the IR.
     ChangeStatus LocalChange = AA->manifest(*this);
     ManifestChange = ManifestChange | LocalChange;
 
     NumAtFixpoint++;
     NumManifested += (LocalChange == ChangeStatus::CHANGED);
   }
 
ChangeStatus Attributor::run() {
   }
 
   NumAttributesManifested += NumManifested;
   NumAttributesValidFixpoint += NumAtFixpoint;
 
   return ManifestChange;
 }
 
+/// Create a shallow wrapper for \p F such that \p F has internal linkage
+/// afterwards. The wrapper will assume the name of \p F and \p F will get a
+/// prefix.
+///
+/// A shallow wrapper is a function with the same type (and attributes) as \p F
+/// that will only call \p F and return the result, if any.
+///
+/// Assuming the declaration of looks like:
+///   rty F(aty0 arg0, ..., atyN argN);
+///
+/// The wrapper will then look as follows:
+///   rty wrapper(aty0 arg0, ..., atyN argN) {
+///     return F(arg0, ..., argN);
+///   }
+///
+static void createShallowWrapper(Function &F) {
+  assert(AllowShallowWrappers &&
+         "Cannot create a wrapper if it is not allowed!");
+  assert(!F.isDeclaration() && "Cannot create a wrapper around a declaration!");
+
+  Module &M = *F.getParent();
+  LLVMContext &Ctx = M.getContext();
+  FunctionType *FnTy = F.getFunctionType();
+
+  StringRef Prefix = "__internal_";
+  F.setName(Prefix + F.getName());
+  Function *Wrapper =
+      Function::Create(FnTy, F.getLinkage(), F.getAddressSpace(),
+                       F.getName().substr(Prefix.size()));
+  M.getFunctionList().insert(F.getIterator(), Wrapper);
+
+  F.setLinkage(GlobalValue::InternalLinkage);
+
+  F.replaceAllUsesWith(Wrapper);
+  assert(F.getNumUses() == 0 && "Uses remained after wrapper was created!");
+
+  // Move the COMDAT section to the wrapper.
+  // TODO: Check if we need to keep it for F as well.
+  Wrapper->setComdat(F.getComdat());
+  F.setComdat(nullptr);
+
+  // Copy all metadata and attributes but keep them on F as well.
+  SmallVector<std::pair<unsigned, MDNode *>, 1> MDs;
+  F.getAllMetadata(MDs);
+  for (auto MDIt : MDs)
+    Wrapper->addMetadata(MDIt.first, *MDIt.second);
+  Wrapper->setAttributes(F.getAttributes());
+
+  // Create the call in the wrapper.
+  BasicBlock *EntryBB = BasicBlock::Create(Ctx, "entry", Wrapper);
+
+  SmallVector<Value *, 8> Args;
+  auto FArgIt = F.arg_begin();
+  for (Argument &Arg : Wrapper->args()) {
+    Args.push_back(&Arg);
+    Arg.setName((FArgIt++)->getName());
+  }
+
+  CallInst *CI = CallInst::Create(&F, Args, "", EntryBB);
+  CI->setTailCall(true);
+  ReturnInst::Create(Ctx, CI->getType()->isVoidTy() ? nullptr : CI, EntryBB);
+
+  NumFnShallowWrapperCreated++;
+}
+
+bool Attributor::mayDependOnNonExactDefinition(
+    AbstractAttribute &AA, SmallPtrSetImpl<Function *> &WrappedFunctions,
+    QueryMapTy &DepMap, DenseMap<AbstractAttribute *, Optional<bool>> &Cache) {
+  Function &AnchorScope = AA.getAnchorScope();
+  bool MayDepend = false;
+  bool NeedShallowWrapper = false;
+
+  // Even if this attribute does not depend on another one, we require a shallow
+  // wrapper if manifesting it will modify the interface of a non-exact
+  // definition.
+  if (!AnchorScope.hasExactDefinition() &&
+      AA.getManifestPosition() != AbstractAttribute::MP_CALL_SITE_ARGUMENT)
+    NeedShallowWrapper = true;
+
+  // Pre-initialize the cache for recursive dependences.
+  Cache[&AA] = false;
+
+  auto &SourceAAs = DepMap[&AA];
+  for (AbstractAttribute *SourceAA : SourceAAs) {
+    // Self dependences and dependences on an invalid abstract attribute are OK.
+    if (SourceAA == &AA || !SourceAA->getState().isValidState())
+      continue;
+
+    // If the anchor scope (=surrounding function) is different and the queried
+    // abstract attribute is in an non-exact definition the current abstract
+    // attribute may depend on that non-exact definition.
+    Function &QuerriedAnchorScope = SourceAA->getAnchorScope();
+    if (&QuerriedAnchorScope != &AnchorScope &&
+        (WrappedFunctions.count(&QuerriedAnchorScope) ||
+         !QuerriedAnchorScope.hasExactDefinition())) {
+      MayDepend = true;
+      break;
+    }
+
+    // Check the cache or recurs if necessary to determine if the queried
+    // abstract attribute might be dependent on non-exact definitions.
+    Optional<bool> QuerriedAARes = Cache[SourceAA];
+    if (!QuerriedAARes.hasValue())
+      QuerriedAARes = mayDependOnNonExactDefinition(*SourceAA, WrappedFunctions,
+                                                    DepMap, Cache);
+
+    assert(QuerriedAARes.hasValue());
+    if (QuerriedAARes.getValue()) {
+      MayDepend = true;
+      break;
+    }
+
+    // We can depend on call site attributes. Note that we set
+    // NeedShallowWrapper already in the beginning if this is a function level
+    // attribute in a non-exact definition.
+    if (SourceAA->getManifestPosition() ==
+        AbstractAttribute::MP_CALL_SITE_ARGUMENT)
+      continue;
+
+    // A dependence to an abstract attribute in this function was found. This
+    // means we need to create a shallow wrapper if the function does not have
+    // an exact definition.
+    NeedShallowWrapper = !AnchorScope.hasExactDefinition();
+  }
+
+  if (NeedShallowWrapper && !AllowShallowWrappers)
+    MayDepend = true;
+
+  if (!MayDepend && NeedShallowWrapper) {
+    WrappedFunctions.insert(&AnchorScope);
+    createShallowWrapper(AnchorScope);
+  }
+
+  if (MayDepend)
+    NumAttrsRequiredDeepWrapper++;
+
+  Cache[&AA] = MayDepend;
+  return MayDepend;
+}
+
 void Attributor::identifyDefaultAbstractAttributes(
     Function &F, InformationCache &InfoCache,
     DenseSet</* Attribute::AttrKind */ unsigned> *Whitelist) {
 
+  auto OnWhiteList = [Whitelist](unsigned ID) -> bool {
+    return Whitelist ? Whitelist->count(ID) : true;
+  };
+
   // Return attributes are only appropriate if the return type is non void.
   Type *ReturnType = F.getReturnType();
   if (!ReturnType->isVoidTy()) {
     // Argument attribute "returned" --- Create only one per function even
     // though it is an argument attribute.
-    if (!Whitelist || Whitelist->count(AAReturnedValues::ID))
+    if (OnWhiteList(AAReturnedValues::ID))
       registerAA(*new AAReturnedValuesImpl(F, InfoCache));
   }
 
   // For each argument we check if we can derive attributes.
   for (Argument &Arg : F.args()) {
 
     // So far only pointer arguments are interesting. However, "returned"
     // is also derived but as a "function return attribute" (see above).
LLVM_DEBUG(dbgs() << "[Attributor] Run on module with " << M.size()
                     << " functions.\n");
 
   // Create an Attributor and initially empty information cache that is filled
   // while we identify default attribute opportunities.
   Attributor A;
   InformationCache InfoCache;
 
   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()) {
-      NumFnWithoutExactDefinition++;
-      continue;
-    }
-
-    // For now we ignore naked and optnone functions.
+    // For now we ignore naked and optnone functions as well as declarations.
     if (F.hasFnAttribute(Attribute::Naked) ||
-        F.hasFnAttribute(Attribute::OptimizeNone))
+        F.hasFnAttribute(Attribute::OptimizeNone) || F.isDeclaration())
       continue;
 
+    if (!F.hasExactDefinition())
+      NumFnWithoutExactDefinition++;
+    else
       NumFnWithExactDefinition++;
 
     // Populate the Attributor with abstract attribute opportunities in the
     // function and the information cache with IR information.
     A.identifyDefaultAbstractAttributes(F, InfoCache);
   }
 
   return A.run() == ChangeStatus::CHANGED;
 }

llvm/test/Transforms/FunctionAttrs/arg_returned.ll

 ; RUN: opt -functionattrs -S < %s | FileCheck %s --check-prefix=FNATTR
 ; RUN: opt -attributor -attributor-disable=false -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
+; RUN: opt -attributor -attributor-disable=false -attributor-allow-shallow-wrappers -S < %s | FileCheck %s --check-prefixes=ATTRIBUTOR,ATTRIBUTOR_WRAPPER
 ; RUN: opt -attributor -attributor-disable=false -functionattrs -S < %s | FileCheck %s --check-prefix=BOTH
 ;
 ; Test cases specifically designed for the "returned" argument attribute.
 ; We use FIXME's to indicate problems and missing attributes.
 ;
 
 ; TEST SCC test returning an integer value argument
 ;

 ;
 ;  int *calls_maybe_redefined_fn(int *r) {
 ;    maybe_redefined_fn(r);
 ;    return r;
 ;  }
 ;
 ; Verify the maybe-redefined function is not annotated:
 ;
-; CHECK: Function Attrs: noinline nounwind uwtable
-; CHECK: define linkonce_odr i32* @maybe_redefined_fn(i32* %r)
+; ATTRIBUTOR: Function Attrs: noinline nounwind uwtable
+; ATTRIBUTOR: define linkonce_odr i32* @maybe_redefined_fn(i32* %r)
+; ATTRIBUTOR_WRAPPER-NEXT: entry:
+; ATTRIBUTOR_WRAPPER-NEXT: %0 = tail call i32* @__internal_maybe_redefined_fn(i32* %r)
+; ATTRIBUTOR_WRAPPER-NEXT: ret i32* %0
+; ATTRIBUTOR_WRAPPER-NEXT: }
+
+; ATTRIBUTOR_WRAPPER: Function Attrs: noinline nounwind uwtable
+; ATTRIBUTOR_WRAPPER: define internal i32* @__internal_maybe_redefined_fn(i32* returned "no-capture-maybe-returned" %r)
+; ATTRIBUTOR_WRAPPER-NEXT: entry:
+; ATTRIBUTOR_WRAPPER-NEXT: ret i32* %r
+; ATTRIBUTOR_WRAPPER-NEXT: }
 ;
-; CHECK: Function Attrs: noinline nounwind uwtable
-; CHECK: define i32* @calls_maybe_redefined_fn(i32* returned %r)
+; ATTRIBUTOR: Function Attrs: noinline nounwind uwtable
+; ATTRIBUTOR: define i32* @calls_maybe_redefined_fn(i32* returned %r)
 ;
 ; BOTH: Function Attrs: noinline nounwind uwtable
 ; BOTH-NEXT: define linkonce_odr i32* @maybe_redefined_fn(i32* %r)
 ;
 ; BOTH: Function Attrs: noinline nounwind uwtable
 ; BOTH-NEXT: define i32* @calls_maybe_redefined_fn(i32* returned %r)
 define linkonce_odr i32* @maybe_redefined_fn(i32* %r) #0 {
 entry:

 ;
 ; Verify the maybe-redefined function is not annotated:
 ;
 ; BOTH: Function Attrs: noinline nounwind uwtable
 ; BOTH-NEXT: define linkonce_odr i32* @maybe_redefined_fn2(i32* %r)
 ; BOTH: Function Attrs: noinline nounwind uwtable
 ; BOTH-NEXT: define i32* @calls_maybe_redefined_fn2(i32* %r)
 ;
+; ATTRIBUTOR: define linkonce_odr i32* @maybe_redefined_fn2(i32* %r)
+; ATTRIBUTOR_WRAPPER: define internal i32* @__internal_maybe_redefined_fn2(i32* returned "no-capture-maybe-returned" %r)
 ; FNATTR:     define i32* @calls_maybe_redefined_fn2(i32* %r)
 ; ATTRIBUTOR: define i32* @calls_maybe_redefined_fn2(i32* %r)
 define linkonce_odr i32* @maybe_redefined_fn2(i32* %r) #0 {
 entry:
   ret i32* %r
 }
 
 define i32* @calls_maybe_redefined_fn2(i32* %r) #0 {