[Attributor] Deduce attributes for non-exact functions

[bbn]

Summary

This patch is based on D63312 and D63319. For now we create shallow wrappers for all functions that are IPO amendable.
See also this github issue.

[bbn]

Updated createShallowWrapper() function

[lebedev.ri]

Tests missnig

[bbn]

• Added tests
• Added noinline attribute to call site to prevent the internal function being inlined

[jdoerfert]

llvm/test/Transforms/Attributor/wrapper.ll

1 ↗

(On Diff #251359)

run it with the new pass manager as well -passes=attributor and verify we set the norecurse attribute on the wrapped functions.

16 ↗

(On Diff #251359)

add the name of the function, even if it is @0 or something. Here and above at the call site.

[bbn]

llvm/test/Transforms/Attributor/wrapper.ll

1 ↗

(On Diff #251359)

did you mean -passes=attributor-cgscc ?

[bbn]

• update tests

[jdoerfert]

Two nits, otherwise LGTM.

llvm/lib/Transforms/IPO/Attributor.cpp

62

NumAttrsRequiredDeepWrapper is not used, or is it?

llvm/test/Transforms/Attributor/wrapper.ll

5 ↗

(On Diff #251561)

Add a CHECK-NOT: Function Attrs before this line.

1 ↗

(On Diff #251359)

Yes, sorry for the confusion.

[jdoerfert]

Adjust the commit title please.

[bbn]

• added some CHECK-NOT to tests
• deleted unused statistic

[jdoerfert]

Apologies for the delay, can you rebase this and provide me with "Firstname Lastname <email>" from you so I can attribute it to you?

[bbn]

Rebased patch.

[bbn]

In D76404#1956336, @jdoerfert wrote:

Apologies for the delay, can you rebase this and provide me with "Firstname Lastname <email>" from you so I can attribute it to you?

"Luofan Chen <[e-mail]>"

Thanks!

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(NumAttributesFixedDueToRequiredDependences,
           "Number of abstract attributes fixed due to required dependences");
static cl::opt<unsigned> DepRecInterval(
     cl::init(4));
 
 static cl::opt<bool> EnableHeapToStack("enable-heap-to-stack-conversion",
                                        cl::init(true), cl::Hidden);
 
 static cl::opt<int> MaxHeapToStackSize("max-heap-to-stack-size", cl::init(128),
                                        cl::Hidden);
 
+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;
else
     AttrList = getAssociatedFunction()->getAttributes();
 
   bool HasAttr = AttrList.hasAttribute(getAttrIdx(), AK);
   if (HasAttr)
     Attrs.push_back(AttrList.getAttribute(getAttrIdx(), AK));
   return HasAttr;
 }
 
-
 void IRPosition::verify() {
   switch (KindOrArgNo) {
   default:
     assert(KindOrArgNo >= 0 && "Expected argument or call site argument!");
     assert((isa<CallBase>(AnchorVal) || isa<Argument>(AnchorVal)) &&
            "Expected call base or argument for positive attribute index!");
     if (isa<Argument>(AnchorVal)) {
       assert(cast<Argument>(AnchorVal)->getArgNo() == unsigned(getArgNo()) &&
ChangeStatus manifest(Attributor &A) override {
     // of the privatizable type are loaded prior to the call and passed to the
     // new function version.
     Attributor::ArgumentReplacementInfo::ACSRepairCBTy ACSRepairCB =
         [=](const Attributor::ArgumentReplacementInfo &ARI,
             AbstractCallSite ACS, SmallVectorImpl<Value *> &NewArgOperands) {
           createReplacementValues(
               PrivatizableType.getValue(), ACS,
               ACS.getCallArgOperand(ARI.getReplacedArg().getArgNo()),
-                                  NewArgOperands);
         };
 
     // Collect the types that will replace the privatizable type in the function
     // signature.
     SmallVector<Type *, 16> ReplacementTypes;
     identifyReplacementTypes(PrivatizableType.getValue(), ReplacementTypes);
 
     // Register a rewrite of the argument.
errs() << "\n[Attributor] Fixpoint iteration done after: "
            << " iterations\n";
     llvm_unreachable("The fixpoint was not reached with exactly the number of "
                      "specified iterations!");
   }
 
   return ManifestChange;
 }
 
+/// Create a shallow wrapper for \p F such that \p F has internal linkage
+/// afterwards. It also sets the original \p F 's name to anonymous
+///
+/// A 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(!F.isDeclaration() && "Cannot create a wrapper around a declaration!");
+
+  Module &M = *F.getParent();
+  LLVMContext &Ctx = M.getContext();
+  FunctionType *FnTy = F.getFunctionType();
+
+  Function *Wrapper =
+      Function::Create(FnTy, F.getLinkage(), F.getAddressSpace(), F.getName());
+  F.setName(""); // set the inside function anonymous
+  M.getFunctionList().insert(F.getIterator(), Wrapper);
+
+  F.setLinkage(GlobalValue::InternalLinkage);
+
+  // 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::isValidFunctionSignatureRewrite(
     Argument &Arg, ArrayRef<Type *> ReplacementTypes) {
 
   auto CallSiteCanBeChanged = [](AbstractCallSite ACS) {
     // Forbid must-tail calls for now.
     return !ACS.isCallbackCall() && !ACS.getCallSite().isMustTailCall();
   };
 
static bool runAttributorOnFunctions(InformationCache &InfoCache,
 
   // Create an Attributor and initially empty information cache that is filled
   // while we identify default attribute opportunities.
   Attributor A(Functions, InfoCache, CGUpdater, DepRecInterval);
 
   for (Function *F : Functions)
     A.initializeInformationCache(*F);
 
+  // Create shallow wrappers for all functions that are not IPO amendable
+  if (AllowShallowWrappers)
+    for (Function *F : Functions)
+      if (!A.isFunctionIPOAmendable(*F))
+        createShallowWrapper(*F);
+
   for (Function *F : Functions) {
     if (F->hasExactDefinition())
       NumFnWithExactDefinition++;
     else
       NumFnWithoutExactDefinition++;
 
     // We look at internal functions only on-demand but if any use is not a
     // direct call or outside the current set of analyzed functions, we have to