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

[Owen Anderson]

On Sep 24, 2012, at 12:55 PM, Evan Cheng <evan.cheng at apple.com> wrote:

> 
> On Sep 24, 2012, at 9:46 AM, Michael Ilseman <milseman at apple.com> wrote:
> 
>> 
>> On Sep 22, 2012, at 3:01 AM, Duncan Sands <baldrick at free.fr> wrote:
>>> urgh, I really don't like creating an instruction just to look it up, then
>>> deleting it again.  Can you make the lookup logic more flexible instead?
>>> By the way, how GVN takes care of commutativity is: when computing the value
>>> number for an instruction, it sorts the operands of commutative instructions
>>> (its sorts the operands according to their value number) before calculating
>>> a value number from them, ensuring that if two commutative instructions are
>>> the same except for a different operand order then they get the same value
>>> number.  I don't know how EarlyCSE works, but maybe something along these lines
>>> would also be possible?
>>> 
>> 
>> Thanks for the feedback!
>> 
>> I also don't like making a new instruction, but considered it the lesser of evils. Essentially, EarlyCSE uses a DenseMap over the instructions, but overloading hash to hash the instruction itself rather than its address. It also overloads isEqual to call Instruction::isIdenticalTo.
>> 
>> As far as changing the lookup mechanism, that would involve both sorting operand hashes on getHashValue, and extending Instruction::isIdenticalToWhenDefined to also calculate equality modulo commutativity (or, essentially inlining its body into the overloaded isEqual function rather than making the call). I don't know what is more desirable, but the original patch to EarlyCSE at least kept it very localized and would only trigger when a commutative op was encountered.
>> 
>> Another option is to restructure EarlyCSE to do expression numbering similarly to GVN, as you mentioned. Again, I don't know if this is more desirable over a localized call to clone(), as this would require a re-encapsulation of Instruction's data that would affect all of EarlyCSE.
> 
> I would strong advise against the new instruction approach. That's considered a definite no-no in LLVM. Do you have a good sense how much effort it is to implement expression numbering?

It's not so hard.  GVN already does it, and the code is relatively independent of the details of GVN.  You could probably extract it to a set of utility files and share it between both passes.

--Owen