[PATCH] D58242: Teach instcombine about remaining idemptotent atomicrmw types

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp:53
+Instruction *InstCombiner::visitAtomicRMWInst(AtomicRMWInst &RMWI) {
+  if (!isIdempotentRMW(RMWI))
+    return nullptr;
----------------
qcolombet wrote:
> spatel wrote:
> > qcolombet wrote:
> > > IIRC to be pedantic idempotent is when something doesn't change when multiplied by itself, hence this name could be misleading (though I could remember wrong :P).
> > Idempotent is (copied from Instruction.h):
> >   ///   Idempotent operators satisfy:  x op x === x
> >   ///
> >   /// In LLVM, the And and Or operators are idempotent.
> > 
> > We have similar logic/switch for binop instructions. See:
> > ConstantExpr::getBinOpIdentity()
> > (not sure if there's a way to share code between this and that)
> Thanks for the reference Sanjay.
> Technically, this does not match what we are doing here: `x op cst === x` where cst may be different than x,  but I guess this is a wildly spread language abuse that people just get it.
Seems like there's consensus the name is confusing.  Given we use the same naming in AtomicExpand, I'm going to land as is, and then change everything in one go.  Any suggestions on better names?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D58242