<html><head><meta http-equiv="Content-Type" content="text/html charset=windows-1252"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; ">What do you propose that could address this efficiently? The only reasonable property I can think of is a visit count when the operand is an Instruction, the constant value when a Constant, and the parameter position when an Argument.<div><br><div><div>On Oct 1, 2012, at 1:15 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com">chandlerc@google.com</a>> wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div class="gmail_extra"><div class="gmail_quote">On Mon, Oct 1, 2012 at 1:14 PM, Michael Ilseman <span dir="ltr"><<a href="mailto:milseman@apple.com" target="_blank" class="cremed">milseman@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">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.</div>
</blockquote><div><br></div><div>The hash codes will be stable within a single execution. My concern is ensuring determinism across executions.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div style="word-wrap:break-word"><div class="h5"><br><div><div>On Oct 1, 2012, at 1:00 PM, Chandler Carruth <<a href="mailto:chandlerc@google.com" target="_blank" class="cremed">chandlerc@google.com</a>> wrote:</div>
<br><blockquote type="cite"><div class="gmail_extra"><div class="gmail_quote">On Mon, Oct 1, 2012 at 11:07 AM, Michael Ilseman <span dir="ltr"><<a href="mailto:milseman@apple.com" target="_blank" class="cremed">milseman@apple.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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).</blockquote>
<div><br></div><div>This sounds very risky. Please don't rely on the particular values of hashes to produce different output.</div><div><br></div><div>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.</div>
<div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> 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.<br>
<div><br>
On Oct 1, 2012, at 11:03 AM, Owen Anderson <<a href="mailto:resistor@mac.com" target="_blank" class="cremed">resistor@mac.com</a>> wrote:<br>
<br>
> 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…<br>
><br>
> --Owen<br>
><br>
> On Oct 1, 2012, at 11:00 AM, Michael Ilseman <<a href="mailto:milseman@apple.com" target="_blank" class="cremed">milseman@apple.com</a>> wrote:<br>
><br>
>> Would also modifying DenseMapInfo<SimpleValue>::getHashValue(SimpleValue Val) resolve the hashing issue?<br>
>><br>
>> On Oct 1, 2012, at 10:56 AM, Owen Anderson <<a href="mailto:resistor@mac.com" target="_blank" class="cremed">resistor@mac.com</a>> wrote:<br>
>><br>
>>><br>
>>> On Oct 1, 2012, at 10:42 AM, Chris Lattner <<a href="mailto:clattner@apple.com" target="_blank" class="cremed">clattner@apple.com</a>> wrote:<br>
>>><br>
>>>><br>
>>>> On Sep 30, 2012, at 7:07 PM, Michael Ilseman <<a href="mailto:milseman@apple.com" target="_blank" class="cremed">milseman@apple.com</a>> wrote:<br>
>>>><br>
>>>>> Attached is a new version. I went with "CanonicalExpr" and added function level comments.<br>
>>>><br>
>>>> Hi Michael,<br>
>>>><br>
>>>> Why is this needed? Why isn't it enough to change the predicate in "DenseMapInfo<SimpleValue>::isEqual" to handle commutative operations?<br>
>>><br>
>>> 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.<br>
>>><br>
>>> --Owen<br>
>><br>
><br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank" class="cremed">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></blockquote></div><br></div>
</blockquote></div><br></div></div></blockquote></div><br></div>
</blockquote></div><br></div></body></html>