[llvm] r261203 - [PM] Port the PostOrderFunctionAttrs pass to the new pass manager and
Chandler Carruth via llvm-commits
llvm-commits at lists.llvm.org
Thu Feb 18 13:48:44 PST 2016
On Thu, Feb 18, 2016 at 1:08 PM Xinliang David Li <xinliangli at gmail.com>
wrote:
> On Thu, Feb 18, 2016 at 12:39 PM, Chandler Carruth <chandlerc at gmail.com>
> wrote:
>
>> On Thu, Feb 18, 2016 at 12:03 PM Xinliang David Li <xinliangli at gmail.com>
>> wrote:
>>
>>> On Thu, Feb 18, 2016 at 3:03 AM, Chandler Carruth via llvm-commits <
>>> llvm-commits at lists.llvm.org> wrote:
>>>
>>>> -bool PostOrderFunctionAttrs::runOnSCC(CallGraphSCC &SCC) {
>>>> +PreservedAnalyses
>>>> +PostOrderFunctionAttrsPass::run(LazyCallGraph::SCC &C,
>>>> CGSCCAnalysisManager *AM) {
>>>> + Module &M = *C.begin()->getFunction().getParent();
>>>> + const ModuleAnalysisManager &MAM =
>>>> + AM->getResult<ModuleAnalysisManagerCGSCCProxy>(C).getManager();
>>>> + FunctionAnalysisManager &FAM =
>>>> + AM->getResult<FunctionAnalysisManagerCGSCCProxy>(C).getManager();
>>>> +
>>>> + // FIXME: Need some way to make it more reasonable to assume that
>>>> this is
>>>> + // always cached.
>>>> + TargetLibraryInfo &TLI =
>>>> *MAM.getCachedResult<TargetLibraryAnalysis>(M);
>>>> +
>>>> + // We pass a lambda into functions to wire them up to the analysis
>>>> manager
>>>> + // for getting function analyses.
>>>> + auto AARGetter = [&](Function &F) -> AAResults & {
>>>> + return FAM.getResult<AAManager>(F);
>>>> + };
>>>> +
>>>> + // Fill SCCNodes with the elements of the SCC. Also track whether
>>>> there are
>>>> + // any external or opt-none nodes that will prevent us from
>>>> optimizing any
>>>> + // part of the SCC.
>>>> + SCCNodeSet SCCNodes;
>>>> + bool HasUnknownCall = false;
>>>> + for (LazyCallGraph::Node &N : C) {
>>>> + Function &F = N.getFunction();
>>>> + if (F.hasFnAttribute(Attribute::OptimizeNone)) {
>>>> + // Treat any function we're trying not to optimize as if it were
>>>> an
>>>> + // indirect call and omit it from the node set used below.
>>>> + HasUnknownCall = true;
>>>> + continue;
>>>> + }
>>>> + // Track whether any functions in this SCC have an unknown call
>>>> edge.
>>>> + // Note: if this is ever a performance hit, we can common it with
>>>> + // subsequent routines which also do scans over the instructions
>>>> of the
>>>> + // function.
>>>> + if (!HasUnknownCall)
>>>> + for (Instruction &I : instructions(F))
>>>> + if (auto CS = CallSite(&I))
>>>> + if (!CS.getCalledFunction()) {
>>>> + HasUnknownCall = true;
>>>> + break;
>>>> + }
>>>> +
>>>>
>>>
>>> The above code does not seem to exist in the legacy implementation. Is
>>> this a functional change?
>>>
>>
>> It shouldn't be. Specifically, the legacy pass manager uses the CallGraph
>> analysis rather than the LazyCallGraph analysis. These two analyses provide
>> slightly different models.
>>
>> One aspect of the CallGraph analysis is that it puts indirect calls into
>> the call graph where LazyCallGraph doesn't. As a consequence, users of
>> LazyCallGraph need to scan to find them.
>>
>
> I see.
>
>
>>
>> The difference in the design is motivated by the fact that all the code
>> using the call graph ended up scanning the instructions anyways. There was
>> some discussion both on the lists and elsewhere that motivated me to
>> essentially wait to find a case where it was an important optimization to
>> cache the indirect calls (or calls to declared external functions, they are
>> equally unanalyzable) in the graph for efficiency.
>>
>
> CallGraph basically caches the indirect callsite info in the SCC node. Is
> it essential to LCG to avoid this design? From this particular case, it
> seems that the it introduces additional scanning cost (per-function).
>
I mean, as I said, we considered adding it but decided not to. There is
nothing fundamental. While we add a scan here, every routine we're about to
call *also* scans all the instructions of all the functions in the SCC, so
it seemed unlikely to be a compile time performance problem in practice.
If this becomes a problem, the first step is to teach function-attrs to do
a single scan for all of the attribute inference it wants to do instead of
8+ scans. If it is *still* a problem, then it might make sense to try to
cache it somehow.
>
> thanks,
>
> David
>
>
>
>>
>>
>>
>>> Also is it possible to refactor the above and share it with the legacy
>>> path?
>>>
>>
>> I think it would make the code substantially less clear because the two
>> analysis provide different models. I'd rather be able to have the code
>> separate and separately commented precisely to help us ask these kinds of
>> questions. =]
>>
>>
>>>
>>>
>>> thanks,
>>>
>>> David
>>>
>>>
>>>
>>>
>>>> + SCCNodes.insert(&F);
>>>> + }
>>>> +
>>>> + bool Changed = false;
>>>> + Changed |= addReadAttrs(SCCNodes, AARGetter);
>>>> + Changed |= addArgumentAttrs(SCCNodes);
>>>> +
>>>> + // If we have no external nodes participating in the SCC, we can
>>>> deduce some
>>>> + // more precise attributes as well.
>>>> + if (!HasUnknownCall) {
>>>> + Changed |= addNoAliasAttrs(SCCNodes);
>>>> + Changed |= addNonNullAttrs(SCCNodes, TLI);
>>>> + Changed |= removeConvergentAttrs(SCCNodes);
>>>> + Changed |= addNoRecurseAttrs(SCCNodes);
>>>> + }
>>>> +
>>>> + return Changed ? PreservedAnalyses::none() :
>>>> PreservedAnalyses::all();
>>>> +}
>>>> +
>>>> +namespace {
>>>> +struct PostOrderFunctionAttrsLegacyPass : public CallGraphSCCPass {
>>>> + static char ID; // Pass identification, replacement for typeid
>>>> + PostOrderFunctionAttrsLegacyPass() : CallGraphSCCPass(ID) {
>>>> +
>>>> initializePostOrderFunctionAttrsLegacyPassPass(*PassRegistry::getPassRegistry());
>>>> + }
>>>> +
>>>> + bool runOnSCC(CallGraphSCC &SCC) override;
>>>> +
>>>> + void getAnalysisUsage(AnalysisUsage &AU) const override {
>>>> + AU.setPreservesCFG();
>>>> + AU.addRequired<AssumptionCacheTracker>();
>>>> + AU.addRequired<TargetLibraryInfoWrapperPass>();
>>>> + addUsedAAAnalyses(AU);
>>>> + CallGraphSCCPass::getAnalysisUsage(AU);
>>>> + }
>>>> +
>>>> +private:
>>>> + TargetLibraryInfo *TLI;
>>>> +};
>>>> +}
>>>> +
>>>> +char PostOrderFunctionAttrsLegacyPass::ID = 0;
>>>> +INITIALIZE_PASS_BEGIN(PostOrderFunctionAttrsLegacyPass,
>>>> "functionattrs",
>>>> + "Deduce function attributes", false, false)
>>>> +INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)
>>>> +INITIALIZE_PASS_DEPENDENCY(CallGraphWrapperPass)
>>>> +INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)
>>>> +INITIALIZE_PASS_END(PostOrderFunctionAttrsLegacyPass, "functionattrs",
>>>> + "Deduce function attributes", false, false)
>>>> +
>>>> +Pass *llvm::createPostOrderFunctionAttrsLegacyPass() { return new
>>>> PostOrderFunctionAttrsLegacyPass(); }
>>>> +
>>>> +bool PostOrderFunctionAttrsLegacyPass::runOnSCC(CallGraphSCC &SCC) {
>>>> TLI = &getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();
>>>> bool Changed = false;
>>>>
>>>>
>>>> Modified: llvm/trunk/lib/Transforms/IPO/IPO.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/IPO.cpp?rev=261203&r1=261202&r2=261203&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/Transforms/IPO/IPO.cpp (original)
>>>> +++ llvm/trunk/lib/Transforms/IPO/IPO.cpp Thu Feb 18 05:03:11 2016
>>>> @@ -18,6 +18,7 @@
>>>> #include "llvm/InitializePasses.h"
>>>> #include "llvm/IR/LegacyPassManager.h"
>>>> #include "llvm/Transforms/IPO.h"
>>>> +#include "llvm/Transforms/IPO/FunctionAttrs.h"
>>>>
>>>> using namespace llvm;
>>>>
>>>> @@ -41,7 +42,7 @@ void llvm::initializeIPO(PassRegistry &R
>>>> initializeLowerBitSetsPass(Registry);
>>>> initializeMergeFunctionsPass(Registry);
>>>> initializePartialInlinerPass(Registry);
>>>> - initializePostOrderFunctionAttrsPass(Registry);
>>>> + initializePostOrderFunctionAttrsLegacyPassPass(Registry);
>>>> initializeReversePostOrderFunctionAttrsPass(Registry);
>>>> initializePruneEHPass(Registry);
>>>> initializeStripDeadPrototypesLegacyPassPass(Registry);
>>>> @@ -73,7 +74,7 @@ void LLVMAddDeadArgEliminationPass(LLVMP
>>>> }
>>>>
>>>> void LLVMAddFunctionAttrsPass(LLVMPassManagerRef PM) {
>>>> - unwrap(PM)->add(createPostOrderFunctionAttrsPass());
>>>> + unwrap(PM)->add(createPostOrderFunctionAttrsLegacyPass());
>>>> }
>>>>
>>>> void LLVMAddFunctionInliningPass(LLVMPassManagerRef PM) {
>>>>
>>>> Modified: llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp?rev=261203&r1=261202&r2=261203&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp (original)
>>>> +++ llvm/trunk/lib/Transforms/IPO/PassManagerBuilder.cpp Thu Feb 18
>>>> 05:03:11 2016
>>>> @@ -31,6 +31,7 @@
>>>> #include "llvm/Target/TargetMachine.h"
>>>> #include "llvm/Transforms/IPO.h"
>>>> #include "llvm/Transforms/IPO/ForceFunctionAttrs.h"
>>>> +#include "llvm/Transforms/IPO/FunctionAttrs.h"
>>>> #include "llvm/Transforms/IPO/InferFunctionAttrs.h"
>>>> #include "llvm/Transforms/Scalar.h"
>>>> #include "llvm/Transforms/Vectorize.h"
>>>> @@ -382,7 +383,7 @@ void PassManagerBuilder::populateModuleP
>>>> Inliner = nullptr;
>>>> }
>>>> if (!DisableUnitAtATime)
>>>> - MPM.add(createPostOrderFunctionAttrsPass());
>>>> + MPM.add(createPostOrderFunctionAttrsLegacyPass());
>>>> if (OptLevel > 2)
>>>> MPM.add(createArgumentPromotionPass()); // Scalarize uninlined fn
>>>> args
>>>>
>>>> @@ -577,7 +578,7 @@ void PassManagerBuilder::addLTOOptimizat
>>>> PM.add(createIPSCCPPass());
>>>>
>>>> // Now that we internalized some globals, see if we can hack on them!
>>>> - PM.add(createPostOrderFunctionAttrsPass());
>>>> + PM.add(createPostOrderFunctionAttrsLegacyPass());
>>>> PM.add(createReversePostOrderFunctionAttrsPass());
>>>> PM.add(createGlobalOptimizerPass());
>>>> // Promote any localized global vars.
>>>> @@ -627,7 +628,7 @@ void PassManagerBuilder::addLTOOptimizat
>>>> PM.add(createScalarReplAggregatesPass());
>>>>
>>>> // Run a few AA driven optimizations here and now, to cleanup the
>>>> code.
>>>> - PM.add(createPostOrderFunctionAttrsPass()); // Add nocapture.
>>>> + PM.add(createPostOrderFunctionAttrsLegacyPass()); // Add nocapture.
>>>> PM.add(createGlobalsAAWrapperPass()); // IP alias analysis.
>>>>
>>>> PM.add(createLICMPass()); // Hoist loop invariants.
>>>>
>>>> Modified: llvm/trunk/test/Transforms/FunctionAttrs/readattrs.ll
>>>> URL:
>>>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/FunctionAttrs/readattrs.ll?rev=261203&r1=261202&r2=261203&view=diff
>>>>
>>>> ==============================================================================
>>>> --- llvm/trunk/test/Transforms/FunctionAttrs/readattrs.ll (original)
>>>> +++ llvm/trunk/test/Transforms/FunctionAttrs/readattrs.ll Thu Feb 18
>>>> 05:03:11 2016
>>>> @@ -1,4 +1,5 @@
>>>> ; RUN: opt < %s -functionattrs -S | FileCheck %s
>>>> +; RUN: opt < %s -aa-pipeline=basic-aa
>>>> -passes='require<targetlibinfo>,cgscc(function-attrs)' -S | FileCheck %s
>>>> @x = global i32 0
>>>>
>>>> declare void @test1_1(i8* %x1_1, i8* readonly %y1_1, ...)
>>>>
>>>>
>>>> _______________________________________________
>>>> llvm-commits mailing list
>>>> llvm-commits at lists.llvm.org
>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>
>>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160218/d4e04e57/attachment.html>
More information about the llvm-commits
mailing list