The dominator tree analysis can be preserved easily.
Some other kinds of analysis can probably be preserved
I just wanted to ask if constructing DT in advance is just okay because this pass doesn't even use DT. Isn't it okay to construct DT when DT is really required in a later pass?
I'll check. AFAIK CallSiteSplitting is run after a bunch of passes that require the DT to be constructed already and by saying we do not preserve the DT we force it to be re-computed.
Hi @fhahn , you should just need one additional line in your patch to inform the pass manager to properly preserve DT.
You can use -debug-pass=Executions with opt during testing to verify the order of creation/deletion of the Dominator Tree pass. Here's an example snippet:
[2018-02-12 11:37:22.659888285] 0x1fb5240 Executing Pass 'Dominator Tree Construction' on Function 'test1'... [2018-02-12 11:37:22.659992339] 0x1fb5240 Executing Pass 'Basic Alias Analysis (stateless AA impl)' on Function 'test1'... [2018-02-12 11:37:22.660156849] 0x1fb5240 Executing Pass 'Function Alias Analysis Results' on Function 'test1'... [2018-02-12 11:37:22.660254827] 0x1fb5240 Executing Pass 'Lazy Value Information Analysis' on Function 'test1'... [2018-02-12 11:37:22.660302165] 0x1fb5240 Executing Pass 'Jump Threading' on Function 'test1'... [2018-02-12 11:37:22.661679568] 0x1fb5240 Made Modification 'Jump Threading' on Function 'test1'... [2018-02-12 11:37:22.661739504] 0x1fb5240 Freeing Pass 'Basic Alias Analysis (stateless AA impl)' on Function 'test1'... [2018-02-12 11:37:22.661767275] 0x1fb5240 Freeing Pass 'Function Alias Analysis Results' on Function 'test1'... [2018-02-12 11:37:22.661795430] 0x1fb5240 Freeing Pass 'Lazy Value Information Analysis' on Function 'test1'... [2018-02-12 11:37:22.661840127] 0x1fb5240 Freeing Pass 'Jump Threading' on Function 'test1'... [2018-02-12 11:37:22.661870858] 0x1fb5240 Freeing Pass 'Dominator Tree Construction' on Function 'test1'... [2018-02-12 11:37:22.661904074] 0x1fb5240 Executing Pass 'Module Verifier' on Function 'test1'...
Please use getAnalysisIfAvailable for the old pass manager
Please use getCachedResult for the new pass manager.
That is the correct way to "get an analysis if it is sitting around so we can update it".
The way you've done it now will actually calculate DT if it's not calculated already.
So this looks right to me.
I'm a little worried about how we are having to modify tons and tons of functions to push these through.
When we get to postdominator updates, we'll have to do the same thing.
At some point, if what we are trying to do is preserve analysis, i wonder if we shouldn't be passing analysismanager's around or something.
An example of that is getBestSimplifyQuery, which takes various analysis managers, etc, and extracts the analysis it cares about them.
If we did similar, we could simplify what gets passed through and easily add updating of a new thing.
Thanks for the pointers. Do you think it would make sense to add something generic which takes various analysis managers, etc, and extracts the analysis on demand, for other passes to use too?
Personally, i totally think this is worthwhile and worth spending some time on to prevent the slow proliferation of 10+ argument functions taking analysis results. Because that's the path we are heading down, and that's definitely hard to refactor, change, etc
But i'm also not the person who would do the work (though happy to review).
If you want to look into it, bring it up on the mailing list and see what folks think.