[llvm-commits] [PATCH] Make EarlyCSE understand commutativity

Chandler Carruth chandlerc at google.com
Mon Oct 1 13:15:01 PDT 2012


On Mon, Oct 1, 2012 at 1:14 PM, Michael Ilseman <milseman at apple.com> wrote:

> I could also swap based on the addresses of the operands, which is how GVN
> does it. Swapping itself would still change execution to execution, but it
> will be consistent within a single execution.
>

The hash codes will be stable within a single execution. My concern is
ensuring determinism across executions.


>
> On Oct 1, 2012, at 1:00 PM, Chandler Carruth <chandlerc at google.com> wrote:
>
> On Mon, Oct 1, 2012 at 11:07 AM, Michael Ilseman <milseman at apple.com>wrote:
>
>> When hashing the operands, first generate all the hashes and if it's
>> commutative then put the smaller hash in first (and also swap the predicate
>> for CmpInst).
>
>
> This sounds very risky. Please don't rely on the particular values of
> hashes to produce different output.
>
> The correct way to detect this in a hash table is to use a symmetric hash
> or to use some existing program input to arbitrarily fix the ordering, and
> always hash both and always in that order.
>
>
>> The one thing I'm unsure of with this approach is that SimpleValue seems
>> meant to just be a wrapper around an Instruction pointer, and this is
>> somewhat violating intuition.
>>
>> On Oct 1, 2012, at 11:03 AM, Owen Anderson <resistor at mac.com> wrote:
>>
>> > How would you decide which commutation is the canonical one, without
>> doing full reassociation?  I suppose you could try to find a symmetric hash
>> function…
>> >
>> > --Owen
>> >
>> > On Oct 1, 2012, at 11:00 AM, Michael Ilseman <milseman at apple.com>
>> wrote:
>> >
>> >> Would also modifying
>> DenseMapInfo<SimpleValue>::getHashValue(SimpleValue Val) resolve the
>> hashing issue?
>> >>
>> >> On Oct 1, 2012, at 10:56 AM, Owen Anderson <resistor at mac.com> wrote:
>> >>
>> >>>
>> >>> On Oct 1, 2012, at 10:42 AM, Chris Lattner <clattner at apple.com>
>> wrote:
>> >>>
>> >>>>
>> >>>> On Sep 30, 2012, at 7:07 PM, Michael Ilseman <milseman at apple.com>
>> wrote:
>> >>>>
>> >>>>> Attached is a new version. I went with "CanonicalExpr" and added
>> function level comments.
>> >>>>
>> >>>> Hi Michael,
>> >>>>
>> >>>> Why is this needed?  Why isn't it enough to change the predicate in
>> "DenseMapInfo<SimpleValue>::isEqual" to handle commutative operations?
>> >>>
>> >>> I don't think that's sufficient.  isEqual is used to resolve
>> collisions.  It won't help here, because the commuted instructions will
>> hash to different values up front.  Note that after reassociation we could
>> rely on  commuted (integer) instructions being canonicalized, but that
>> doesn't help given EarlyCSE's place in the pipeline.
>> >>>
>> >>> --Owen
>> >>
>> >
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20121001/9b261771/attachment.html>


More information about the llvm-commits mailing list