[PATCH] D141277: [InstCombine] Don't optimize idempotent `atomicrmw <op>, 0` into `load atomic`

[Ralf via Phabricator via llvm-commits]

RalfJung added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineAtomicRMW.cpp:135
     SI->setAlignment(DL.getABITypeAlign(RMWI.getType()));
     return eraseInstFromFunction(RMWI);
   }
----------------
qcolombet wrote:
> nikic wrote:
> > Just to double check, is this `atomicrmw` -> `store` fold fine, or does it also have a similar issue?
> It might, but I'd like a "language lawyer" to confirm.
> 
> It took me some time to get convinced that this optimization is wrong to begin with so I would rather have an actual expert on the matter to comment :).
> 
> @RalfJung 
I'm afraid I am not a weak memory expert, so I don't know for sure.

But given that RMWs have special semantics that stores don't have, I am at least doubtful of this optimization. I will ask some other people. :)
If I understand the code correctly, this will apply specifically to Xchg whose loaded value is ignored, and whose ordering is Release or Relaxed?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141277