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

[Chandler Carruth]

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/a33c0e80/attachment.html>