[PATCH] D57854: [InstCombine] Optimize `atomicrmw <op>, 0` into `load atomic` when possible

Sat Jan 7 14:59:33 PST 2023

JonChesterfield added a comment.

I believe that replacing `atomicrmw <op> LHS, 0` with `load atomic LHS`. is not sound. There's an extensive discussion in a github issue https://github.com/llvm/llvm-project/issues/56450 which roughly concludes that this patch should be amended or reverted, but it doesn't seem to have tracked across to here.

In particular, anyone programming exclusively using thread fences, relaxed loads and stores, and relaxed atomicrmw, is going to have a bad time when this optimisation kicks in. That's roughly the model that nvptx steers one towards. I can work around this miscompilation by remembering to mark rmw operations as acquire-release instead of relaxed but I'd rather we fix it in trunk instead.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57854