[PATCH] D86798: [EarlyCSE] fold commutable intrinsics

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In D86798#2245138 <https://reviews.llvm.org/D86798#2245138>, @nikic wrote:

> In D86798#2245116 <https://reviews.llvm.org/D86798#2245116>, @spatel wrote:
>
>> Or I can just leave the 'fix' intrinsics off of the list here for now. Do you have a preference?
>
> Yeah, I think it would be best to drop the fixpoint intrinsics for now, as they aren't commutative in the usual sense and we won't be able to exploit that in the current implementation anyway.

Just dropping the fixed-point intrinsics from this patch is not enough because this assert still fires for the 2 operand intrinsics:

  assert(I->getNumOperands() == 2 && "Unsupported commutative instruction!");

...because a CallInst's first operand is the called function (the 2 argument intrinsics always have 3 operands).
So at the least, I need to adjust the asserts in GVN and NewGVN. There's also a use of isCommutative() in SLP that I can't tell at a glance is safe. Still want to make those changes here, or a follow-up patch?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D86798/new/

https://reviews.llvm.org/D86798