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

[Daniel Berlin]

>>
>>
>> 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.


>> 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)