[PATCH] D86798: [EarlyCSE] fold commutable intrinsics

Nikita Popov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 28 11:58:45 PDT 2020


nikic added a comment.

> As noted in https://llvm.org/PR46897 , there is a commutative property specifier for intrinsics, but no corresponding function attribute, and so apparently no uses of that bit. I can remove that here or independently if this is an adequate or better solution.

I think the basic approach here is fine. Dropping the confusing intrinsic property once this lands sounds good.

The only suggestion I would make is to make the primary `Instruction::isCommutative()` method actually forward to `IntrinsicInst::isCommutative()` for intrinsics. Having `I->isCommutative()` and `cast<IntrinsicInst>(I)->isCommutative()` report different things is confusing. I think if you do this, then we will also get commutation support in GVN for free.


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

https://reviews.llvm.org/D86798



More information about the llvm-commits mailing list