[PATCH] Rename determineInsertionPoints to determinePHINodes, expose it so it can be reused
dberlin at dberlin.org
Mon Apr 20 15:21:26 PDT 2015
On Mon, Apr 20, 2015 at 3:07 PM, Quentin Colombet <qcolombet at apple.com> wrote:
>> On Apr 20, 2015, at 2:44 PM, Daniel Berlin <dberlin at dberlin.org> wrote:
>> 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.
> Well, what about providing the list of uses then?
If it wanted to compute it from scratch, it would need the list of
uses and defs it is caring about, the basic blocks they belong to, and
their relative positions (so it knows if def happens before use in a
given block). Otherwise, it would mark things that are defined before
used in a block as live-in when they are not.
This would be O(the size of the entire function) extra storage :)
It could also be done by passing in DefBeforeUse blocks and
UsingBlocks, but at least to me, it seems less user friendly that just
giving the set of live-in blocks.
Happy to do it though if you disagree.
More information about the llvm-commits