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

[Chris Lattner]

On 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.

I think that this is the right way to go.  The code in EarlyCSE should stay really simple, and I don't like the idea of adding yet-another expression abstraction that has to stay up to date with the IR as we add new things to it.


What I'm suggesting is that the hashing code can look like (psuedo code obviously):

unsigned DenseMapInfo<SimpleValue>::getHashValue(SimpleValue Val) {
  Instruction *Inst = Val.Inst;

  // Hash in all of the operands as pointers.
  bool Swapped = false;
  if (isa<BinaryOperator>(Inst) && Inst->isCommutative() && Inst->getOperand(0) > Inst->getOperand(1)) {
    Inst->swapOperands();
    Swapped = true;
  }

  unsigned Res = 0;
  for (unsigned i = 0, e = Inst->getNumOperands(); i != e; ++i)
    Res ^= getHash(Inst->getOperand(i)) << (i & 0xF);

 ...

  // Mix in the opcode.
  Res = (Res << 1) ^ Inst->getOpcode();

  if (Swapped) Inst->swapOperands();
  return Res;
}

Similarly for compares.

Of course, it would be much better to avoid actually swapping the operands, and I don't think it would make the code any more complex to do that.

-Chris