[llvm] 284f039 - [Transforms] Merge function attributes within InlineFunction (NFC)

Kazu Hirata via llvm-commits llvm-commits at lists.llvm.org
Sat Sep 17 23:10:32 PDT 2022


Author: Kazu Hirata
Date: 2022-09-17T23:10:23-07:00
New Revision: 284f0397e2779d9de6613c81539367f9b35d460f

URL: https://github.com/llvm/llvm-project/commit/284f0397e2779d9de6613c81539367f9b35d460f
DIFF: https://github.com/llvm/llvm-project/commit/284f0397e2779d9de6613c81539367f9b35d460f.diff

LOG: [Transforms] Merge function attributes within InlineFunction (NFC)

In the past, we've had a bug resulting in a compiler crash after
forgetting to merge function attributes (D105729).

This patch teaches InlineFunction to merge function attributes.  This
way, we minimize the "time" when the IR is valid, but the function
attributes are not.

Differential Revision: https://reviews.llvm.org/D134117

Added: 
    

Modified: 
    llvm/include/llvm/Transforms/Utils/Cloning.h
    llvm/lib/Transforms/IPO/AlwaysInliner.cpp
    llvm/lib/Transforms/IPO/Inliner.cpp
    llvm/lib/Transforms/IPO/ModuleInliner.cpp
    llvm/lib/Transforms/IPO/SampleProfile.cpp
    llvm/lib/Transforms/Utils/InlineFunction.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Transforms/Utils/Cloning.h b/llvm/include/llvm/Transforms/Utils/Cloning.h
index fdc55bea99e7b..31309a10c0c49 100644
--- a/llvm/include/llvm/Transforms/Utils/Cloning.h
+++ b/llvm/include/llvm/Transforms/Utils/Cloning.h
@@ -260,10 +260,14 @@ class InlineFunctionInfo {
 /// and all varargs at the callsite will be passed to any calls to
 /// ForwardVarArgsTo. The caller of InlineFunction has to make sure any varargs
 /// are only used by ForwardVarArgsTo.
+///
+/// The callee's function attributes are merged into the callers' if
+/// MergeAttributes is set to true.
 InlineResult InlineFunction(CallBase &CB, InlineFunctionInfo &IFI,
                             AAResults *CalleeAAR = nullptr,
                             bool InsertLifetime = true,
-                            Function *ForwardVarArgsTo = nullptr);
+                            Function *ForwardVarArgsTo = nullptr,
+                            bool MergeAttributes = false);
 
 /// Clones a loop \p OrigLoop.  Returns the loop and the blocks in \p
 /// Blocks.

diff  --git a/llvm/lib/Transforms/IPO/AlwaysInliner.cpp b/llvm/lib/Transforms/IPO/AlwaysInliner.cpp
index 58cea7ebb7499..377f224eb0cc7 100644
--- a/llvm/lib/Transforms/IPO/AlwaysInliner.cpp
+++ b/llvm/lib/Transforms/IPO/AlwaysInliner.cpp
@@ -71,7 +71,9 @@ PreservedAnalyses AlwaysInlinerPass::run(Module &M,
             &FAM.getResult<BlockFrequencyAnalysis>(F));
 
         InlineResult Res = InlineFunction(
-            *CB, IFI, &FAM.getResult<AAManager>(F), InsertLifetime);
+            *CB, IFI, &FAM.getResult<AAManager>(F), InsertLifetime,
+            /*ForwardVarArgsTo=*/nullptr,
+            /*MergeAttributes=*/true);
         if (!Res.isSuccess()) {
           ORE.emit([&]() {
             return OptimizationRemarkMissed(DEBUG_TYPE, "NotInlined", DLoc,
@@ -88,9 +90,6 @@ PreservedAnalyses AlwaysInlinerPass::run(Module &M,
             InlineCost::getAlways("always inline attribute"),
             /*ForProfileContext=*/false, DEBUG_TYPE);
 
-        // Merge the attributes based on the inlining.
-        AttributeFuncs::mergeAttributesForInlining(*Caller, F);
-
         Changed = true;
       }
 

diff  --git a/llvm/lib/Transforms/IPO/Inliner.cpp b/llvm/lib/Transforms/IPO/Inliner.cpp
index 36151998a0ad5..d3390e07f11f4 100644
--- a/llvm/lib/Transforms/IPO/Inliner.cpp
+++ b/llvm/lib/Transforms/IPO/Inliner.cpp
@@ -315,15 +315,15 @@ static InlineResult inlineCallIfPossible(
 
   // Try to inline the function.  Get the list of static allocas that were
   // inlined.
-  InlineResult IR = InlineFunction(CB, IFI, &AAR, InsertLifetime);
+  InlineResult IR = InlineFunction(CB, IFI, &AAR, InsertLifetime,
+                                   /*ForwardVarArgsTo=*/nullptr,
+                                   /*MergeAttributes=*/true);
   if (!IR.isSuccess())
     return IR;
 
   if (InlinerFunctionImportStats != InlinerFunctionImportStatsOpts::No)
     ImportedFunctionsStats.recordInline(*Caller, *Callee);
 
-  AttributeFuncs::mergeAttributesForInlining(*Caller, *Callee);
-
   if (!DisableInlinedAllocaMerging)
     mergeInlinedArrayAllocas(Caller, IFI, InlinedArrayAllocas, InlineHistory);
 
@@ -915,7 +915,10 @@ PreservedAnalyses InlinerPass::run(LazyCallGraph::SCC &InitialC,
           &FAM.getResult<BlockFrequencyAnalysis>(Callee));
 
       InlineResult IR =
-          InlineFunction(*CB, IFI, &FAM.getResult<AAManager>(*CB->getCaller()));
+          InlineFunction(*CB, IFI, &FAM.getResult<AAManager>(*CB->getCaller()),
+                         /*InsertLifetime=*/true,
+                         /*ForwardVarArgsTo=*/nullptr,
+                         /*MergeAttributes=*/true);
       if (!IR.isSuccess()) {
         Advice->recordUnsuccessfulInlining(IR);
         continue;
@@ -970,9 +973,6 @@ PreservedAnalyses InlinerPass::run(LazyCallGraph::SCC &InitialC,
         }
       }
 
-      // Merge the attributes based on the inlining.
-      AttributeFuncs::mergeAttributesForInlining(F, Callee);
-
       // For local functions or discardable functions without comdats, check
       // whether this makes the callee trivially dead. In that case, we can drop
       // the body of the function eagerly which may reduce the number of callers

diff  --git a/llvm/lib/Transforms/IPO/ModuleInliner.cpp b/llvm/lib/Transforms/IPO/ModuleInliner.cpp
index 5dc7c777d7c1c..1bc3ba9efa94a 100644
--- a/llvm/lib/Transforms/IPO/ModuleInliner.cpp
+++ b/llvm/lib/Transforms/IPO/ModuleInliner.cpp
@@ -218,7 +218,10 @@ PreservedAnalyses ModuleInlinerPass::run(Module &M,
         &FAM.getResult<BlockFrequencyAnalysis>(Callee));
 
     InlineResult IR =
-        InlineFunction(*CB, IFI, &FAM.getResult<AAManager>(*CB->getCaller()));
+        InlineFunction(*CB, IFI, &FAM.getResult<AAManager>(*CB->getCaller()),
+                       /*InsertLifetime=*/true,
+                       /*ForwardVarArgsTo=*/nullptr,
+                       /*MergeAttributes=*/true);
     if (!IR.isSuccess()) {
       Advice->recordUnsuccessfulInlining(IR);
       continue;
@@ -251,9 +254,6 @@ PreservedAnalyses ModuleInlinerPass::run(Module &M,
       }
     }
 
-    // Merge the attributes based on the inlining.
-    AttributeFuncs::mergeAttributesForInlining(F, Callee);
-
     // For local functions, check whether this makes the callee trivially
     // dead. In that case, we can drop the body of the function eagerly
     // which may reduce the number of callers of other functions to one,

diff  --git a/llvm/lib/Transforms/IPO/SampleProfile.cpp b/llvm/lib/Transforms/IPO/SampleProfile.cpp
index 062c7badca869..a326f238e4ef1 100644
--- a/llvm/lib/Transforms/IPO/SampleProfile.cpp
+++ b/llvm/lib/Transforms/IPO/SampleProfile.cpp
@@ -1221,13 +1221,14 @@ bool SampleProfileLoader::tryInlineCandidate(
 
   InlineFunctionInfo IFI(nullptr, GetAC);
   IFI.UpdateProfile = false;
-  if (!InlineFunction(CB, IFI).isSuccess())
+  InlineResult IR = InlineFunction(CB, IFI,
+                                   /*CalleeAAR=*/nullptr,
+                                   /*InsertLifetime=*/true,
+                                   /*ForwardVarArgsTo=*/nullptr,
+                                   /*MergeAttributes=*/true);
+  if (!IR.isSuccess())
     return false;
 
-  // Merge the attributes based on the inlining.
-  AttributeFuncs::mergeAttributesForInlining(*BB->getParent(),
-                                             *CalledFunction);
-
   // The call to InlineFunction erases I, so we can't pass it here.
   emitInlinedIntoBasedOnCost(*ORE, DLoc, BB, *CalledFunction, *BB->getParent(),
                              Cost, true, getAnnotatedRemarkPassName());

diff  --git a/llvm/lib/Transforms/Utils/InlineFunction.cpp b/llvm/lib/Transforms/Utils/InlineFunction.cpp
index 5abf7b1faae11..1ea73d1e25085 100644
--- a/llvm/lib/Transforms/Utils/InlineFunction.cpp
+++ b/llvm/lib/Transforms/Utils/InlineFunction.cpp
@@ -1786,7 +1786,8 @@ inlineRetainOrClaimRVCalls(CallBase &CB, objcarc::ARCInstKind RVCallKind,
 llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI,
                                         AAResults *CalleeAAR,
                                         bool InsertLifetime,
-                                        Function *ForwardVarArgsTo) {
+                                        Function *ForwardVarArgsTo,
+                                        bool MergeAttributes) {
   assert(CB.getParent() && CB.getFunction() && "Instruction not in function!");
 
   // FIXME: we don't inline callbr yet.
@@ -2509,6 +2510,9 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI,
     // Since we are now done with the return instruction, delete it also.
     Returns[0]->eraseFromParent();
 
+    if (MergeAttributes)
+      AttributeFuncs::mergeAttributesForInlining(*Caller, *CalledFunc);
+
     // We are now done with the inlining.
     return InlineResult::success();
   }
@@ -2672,5 +2676,8 @@ llvm::InlineResult llvm::InlineFunction(CallBase &CB, InlineFunctionInfo &IFI,
     }
   }
 
+  if (MergeAttributes)
+    AttributeFuncs::mergeAttributesForInlining(*Caller, *CalledFunc);
+
   return InlineResult::success();
 }


        


More information about the llvm-commits mailing list