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

[Daniel Berlin]

On Mon, Apr 20, 2015 at 2:29 PM, Quentin Colombet <qcolombet at apple.com> wrote:
> Hi Daniel
>
>
> ================
> Comment at: include/llvm/Transforms/Utils/PromoteMemToReg.h:70
> @@ +69,3 @@
> +                       const DenseMap<DomTreeNode *, unsigned> &DomLevels,
> +                       SmallVectorImpl<std::pair<unsigned, BasicBlock *>> &PHIBlocks);
> +
> ----------------
> From the user stand point I think we should not have to provide the DomLevels and the LiveInBlocks.

The only way to do this would be callback functions, for two reasons:

1 .The live-in-blocks require knowledge of what a def and a use is, at
the very least.
For these two use cases, for memory ssa, this is not the same as ssa
(for MemorySSA, because it's not part of the IR, so we'd have to
provide a way to walk it)

For PromoteMemToReg, it wants live-in blocks for specific alloca's
passed to PromoteMemToReg (in particular, the one is it currently
determining phi's for, since it does it per-alloca).

This also would require a callback for this function to determine.

2.   DomLevels does not change per-variable, and so, both memoryssa
and promotememtoreg only compute it once per function.

If we compute it ourselves here, we will compute it once per variable.


I could wrap this whole thing in a class to solve the second problem.
It would not solve the first.




>
> The BBNumbers are already on your “to-remove” list, so it is fine for now.
>
> http://reviews.llvm.org/D9118
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>