[PATCH] Extend EarlyCSE to handle basic cases from JumpThreading and CVP

[Daniel Berlin]

-review because i don't want it to attach the patch i'm about to send.

FWIW:
I've attached a working implementation of dominated_uses and dominated_edge_uses
It could be a bit prettier (the ends can be made to not need
arguments), and maybe it should be a member function of something.

I also lack the ability to reason about why filter_iterator must have
the using base::operator++ to get operator++(int) exposed from
iterator_facade, when it doesn't seem to be that way for any other
iterator_facade user (or maybe none of them use postfix increment, i
didn't look all that hard), *and* operator != simultaneously seems to
work happily from iterator_facade with nothing else.





On Wed, May 13, 2015 at 5:30 PM, Philip Reames
<listmail at philipreames.com> wrote:
> I'll throw it in Local.h/cpp for the moment, but I may go back and write the
> dominated use iterator.  Doing that as a filter on an iterator range
> shouldn't be too challenging.  I definitely want to do that refactor as it's
> own change set though.
>
> I'll update the patch with the commonned functionality in a moment. What are
> you thoughts otherwise?
>
>
> On 05/13/2015 05:14 PM, Daniel Berlin wrote:
>>
>> This is a good question.
>>
>> I looked through, and i see that it is a common idiom, but often there
>> is a small amount of other code people are doing in the middle or as
>> part of replacement.  For example,
>> Transforms/ObjCARC/ObjCARCContract.cpp:552
>>
>> If it's not too much to ask, i actually think the best option may be
>> something like "a dominated use iterator".
>>
>> IE
>>
>> Instead of replaceAllDominatedUses(From, To, DomRoot)
>>
>> for (auto &Use : dominated_edge_uses(From, DomRoot))
>>    {
>>        // Do whatever extra stuff they  they want
>>       Use.set(To);
>>     }
>>
>>
>> and
>> instead of
>> replaceAllDominatedUses(From, To, From->getParent())
>>
>> for (auto &Use: dominated_uses(From))
>>    Use.set(To);
>>
>>
>> If that's too much, let's just put replaceAllDominatedUses in local.* for
>> now.
>>
>>
>>
>>
>>
>>
>>
>> On Wed, May 13, 2015 at 4:13 PM, Philip Reames
>> <listmail at philipreames.com> wrote:
>>>
>>> ================
>>> Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:490
>>> @@ +489,3 @@
>>> +                  ToUpdate.push_back(IU);
>>> +            for (Instruction *UserI : ToUpdate) {
>>> +              DEBUG(dbgs() << "EarlyCSE CVP: Replace dominated use of '"
>>> ----------------
>>> dberlin wrote:
>>>>
>>>> Can you common this part with GVN's replaceAllDominatedUsesWith?
>>>>
>>>> (IE just move that function somewhere common and use it?)
>>>
>>> Absolutely.  Didn't know that existed.  Would it make sense to put that
>>> directly on Value?  Or would you rather see it as a helper function in
>>> Local.h or someplace similar?
>>>
>>> http://reviews.llvm.org/D9763
>>>
>>> EMAIL PREFERENCES
>>>    http://reviews.llvm.org/settings/panel/emailpreferences/
>>>
>>>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: dominateduseiterator.patch
Type: application/octet-stream
Size: 5729 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150514/fdaae984/attachment.obj>