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

[Michael Ilseman]

You are correct.

On Oct 2, 2012, at 1:02 AM, Duncan Sands <baldrick at free.fr> wrote:

> Hi Michael,
> 
> On 01/10/12 22:14, Michael Ilseman wrote:
>> I could also swap based on the addresses of the operands, which is how GVN does
>> it.
> 
> this isn't how GVN does it.  For commutative instructions GVN sorts the value
> numbers of the operands, not their addresses.
> 
> Ciao, Duncan.
> 
> Swapping itself would still change execution to execution, but it will be
>> consistent within a single execution.
>> 
>> On Oct 1, 2012, at 1:00 PM, Chandler Carruth <chandlerc at google.com
>> <mailto:chandlerc at google.com>> wrote:
>> 
>>> On Mon, Oct 1, 2012 at 11:07 AM, Michael Ilseman <milseman at apple.com
>>> <mailto: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
>>>    <mailto: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
>>>    <mailto: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
>>>    <mailto:resistor at mac.com>> wrote:
>>>    >>
>>>    >>>
>>>    >>> On Oct 1, 2012, at 10:42 AM, Chris Lattner <clattner at apple.com
>>>    <mailto:clattner at apple.com>> wrote:
>>>    >>>
>>>    >>>>
>>>    >>>> On Sep 30, 2012, at 7:07 PM, Michael Ilseman <milseman at apple.com
>>>    <mailto: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 <mailto:llvm-commits at cs.uiuc.edu>
>>>    http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> 
>>> 
>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits