[PATCH] Rename determineInsertionPoints to determinePHINodes, expose it so it can be reused

[Quentin Colombet]

> On Apr 20, 2015, at 5:54 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
> 
>>> 
>>> 
>>> class IDFComputation {
>>> 
>>> // This constructor takes the minimum needed to do a correct
>>> computation without livein
>>> IDFComputation(DT); // defaults to uselivein=false
>>> 
> 
> Ugh, we have to change this to only take DT, and do
> 
> void setDefiningBlocks(DefiningBlocks) as well, unless you have a better idea.

Nope, sounds like the best approach, at least for now :).

Thanks again Daniel!
-Quentin

> 
> 
>>> void setLiveIn(DT, <blocks or lists of uses depending on the above>);
>>> // Computes livein
>>> void setNoLiveIn();  // Changes uselivein to false, frees whatever
>>> livein info  exists
>> 
>> I do not see the benefit in having the ability to change the LiveIn instead of being part of the constructor.
>> Could you elaborate on the use case you have in mind.
>> 
> 
> So, it needs to be able to be reset in between calculations.
> 
> The current user, promotememtoreg, does this right now:
> 
> 
> Create bbnumbers
> Create domlevels
> 
> for each variable to promote:
>  compute defining blocks for variable.
>  compute live in info for variable
>  IDFCalculate with already-computed bbnumbers, domlevels, and
> per-variable live-in info
> 
> 
> Note that it shares bbnumbers/domlevels between calls to IDFCalculate
> (currently called determineInsertionPoint), but recomputes live-in per
> variable.
> 
> The obvious (at least to me) translation of this is:
> 
> 
> IDFCalculator IDF (DT); <takes care of dom levels and bbnumbers>
> 
> for each alloca to promote:
>  compute live in info
>  IDF.setLiveIn(that info)
>  compute defining blocks
>  IDF.setDefiningBlocks(that info)
>  PHIBlocks = IDF.calculate();
> 
> 
> 
> Better API ideas welcome :) (but see below, we can cache the DF
> computation it does internally even if defining blocks changes, as
> long as live-in blocks does not change).
> 
> 
>>> 
>>> void calculate(IDFBlocks as output parameter).  // calculate the IDF.
>>> Internally computes livein, domlevels and bbnumbers if necessary.
>> 
>> Return value instead of output parameter?
>> For RVO.
> 
> Sure.
> 
>> 
>>> 
>>> private:
>>> bool uselivein;
>>> DenseMap DomLevels;
>>> DenseMap BBNumbers;
>>> DominatorTree DT
>>> }
>>> 
>>> 
>>> The reasoning for this API is that
>>> 1. We have users who do not care about livein (it's only necessary to
>>> compute pruned SSA).  MemorySSA does not care to compute live in, nor
>>> do most users (if you wanted to use this to compute SSI or whatever).
>>> It's only useful to compute pruned SSA.
>>> 2. We can cache the result of most of calculate's internal
>>> computations if the set of livein blocks and the CFG does not change
>>> between computations.
>> 
>> But since we cannot change the defining blocks, I guess there is not much to cache between two different users, isn’t?
> 
> Yes, you are right, we should be able to change defining blocks as
> well (because it's subcomputing DF, unless the CFG has changed,
> DF(block) has not changed, *even if* definingblocks changes)