[llvm] r363287 - [clang][NewPM] Fix broken -O0 test from missing assumptions

[Chandler Carruth via llvm-commits]

On Fri, Jun 14, 2019 at 10:56 PM Leonard Chan <leonardchan at google.com>
wrote:

> Thanks for bringing this up. I'm not sure how polly works, but *I think*
> I found out the root of the problem.
>
> For new pass manager runs, the FunctionAnalysisManagerModuleProxy is
> registered in PassBuilder::crossRegisterProxies(). `opt` calls this in
> `llvm::runPassPipeline()` but only if the new PM driver is used (which can
> be triggered by passing `-passes='...'`). The command in question though
> does not seem to be using the new PM, so the problem is that this new PM
> pass is being used in a legacy PM run.
>
> After some digging, it seems that the new PM AlwaysInlinerPass (and some
> other new PM passes) are created and run in
> `polly/trunk/lib/Transform/ScopInliner.cpp` as a part
> of ScopInliner::runOnSCC(), which should be a legacy pass.
>
> I'd double check this with Chandler or Philip, but I imagine the correct
> thing to do is to instead change some of the body
> of ScopInliner::runOnSCC() to 1) use the legacy PM classes (like
> AlwaysInlinerLegacyPass), and 2) change ScopInliner to have the
> AlwaysInliner as a pass dependency instead of invoking it directly in the
> run.
>

Or 3) register and set up the analysis manager inside of the ScopInliner.

Nothing goes wrong by nesting new PM inside of something else. You just
have to build and set up the analysis manager correctly to hand to the
underlying pass.

FWIW, I agree w/ your analysis of the problem and the available solutions.
I don't have a strong opinion. It'd be good to land one of these ore
temporarily revert until we figure out what to do.... #3 is honestly
probably the minimal change -- it should allow things to work exactly as
they did before.


>
> - Leonard
>
> On Fri, Jun 14, 2019 at 9:20 PM Michael Kruse <llvm-commits at meinersbur.de>
> wrote:
>
>> This made two regression tests in Polly crash:
>>
>> Debug\bin\opt.exe  -polly-process-unprofitable  -polly-remarks-minimal
>>  -polly-use-llvm-names
>>
>> -polly-import-jscop-dir=C:\Users\meinersbur\src\llvm\tools\polly\test\ScopInliner
>>  -polly-codegen-verify  -polly-detect-full-functions
>> -polly-scop-inliner  -polly-scops -analyze
>> -polly-invariant-load-hoisting
>> Assertion failed: AnalysisPasses.count(PassT::ID()) && "This analysis
>> pass was not registered prior to being queried", file
>> C:\Users\meinersbur\src\llvm\include\llvm/IR/PassManager.h, line 778
>>
>> with PassT being FunctionAnalysisManagerModuleProxy. Where is this
>> pass supposed to be registered?
>>
>> Michael
>>
>>
>> Am Do., 13. Juni 2019 um 13:15 Uhr schrieb Leonard Chan via
>> llvm-commits <llvm-commits at lists.llvm.org>:
>> >
>> > Author: leonardchan
>> > Date: Thu Jun 13 11:18:40 2019
>> > New Revision: 363287
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=363287&view=rev
>> > Log:
>> > [clang][NewPM] Fix broken -O0 test from missing assumptions
>> >
>> > Add an AssumptionCache callback to the InlineFuntionInfo used for the
>> > AlwaysInlinerPass to match codegen of the AlwaysInlinerLegacyPass to
>> generate
>> > llvm.assume. This fixes CodeGen/builtin-movdir.c when new PM is enabled
>> by
>> > default.
>> >
>> > Differential Revision: https://reviews.llvm.org/D63170
>> >
>> > Modified:
>> >     llvm/trunk/lib/Transforms/IPO/AlwaysInliner.cpp
>> >
>> > Modified: llvm/trunk/lib/Transforms/IPO/AlwaysInliner.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/IPO/AlwaysInliner.cpp?rev=363287&r1=363286&r2=363287&view=diff
>> >
>> ==============================================================================
>> > --- llvm/trunk/lib/Transforms/IPO/AlwaysInliner.cpp (original)
>> > +++ llvm/trunk/lib/Transforms/IPO/AlwaysInliner.cpp Thu Jun 13 11:18:40
>> 2019
>> > @@ -31,8 +31,17 @@ using namespace llvm;
>> >
>> >  #define DEBUG_TYPE "inline"
>> >
>> > -PreservedAnalyses AlwaysInlinerPass::run(Module &M,
>> ModuleAnalysisManager &) {
>> > -  InlineFunctionInfo IFI;
>> > +PreservedAnalyses AlwaysInlinerPass::run(Module &M,
>> > +                                         ModuleAnalysisManager &MAM) {
>> > +  // Add inline assumptions during code generation.
>> > +  FunctionAnalysisManager &FAM =
>> > +
>> MAM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
>> > +  std::function<AssumptionCache &(Function &)> GetAssumptionCache =
>> > +      [&](Function &F) -> AssumptionCache & {
>> > +    return FAM.getResult<AssumptionAnalysis>(F);
>> > +  };
>> > +  InlineFunctionInfo IFI(/*cg=*/nullptr, &GetAssumptionCache);
>> > +
>> >    SmallSetVector<CallSite, 16> Calls;
>> >    bool Changed = false;
>> >    SmallVector<Function *, 16> InlinedFunctions;
>> >
>> >
>> > _______________________________________________
>> > llvm-commits mailing list
>> > llvm-commits at lists.llvm.org
>> > https://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/20190615/0ffe18ae/attachment.html>