[PATCH] D72995: [MLIR] LLVM Dialect: add llvm.cmpxchg and improve llvm.atomicrmw custom parser

[Frank Laub via Phabricator via llvm-commits]

flaub marked an inline comment as done.
flaub added inline comments.


================
Comment at: mlir/test/Dialect/LLVMIR/invalid.mlir:499
+func @cmpxchg_success_must_be_stronger(%i32_ptr : !llvm<"i32*">, %i32 : !llvm.i32) {
+  // expected-error at +1 {{failure ordering must be no stronger than success ordering}}
+  %0 = llvm.cmpxchg %i32_ptr, %i32, %i32 monotonic acquire : !llvm.i32
----------------
jfb wrote:
> That restriction came from C++11 and is no longer in place in C++17:
> http://wg21.link/P0418
> See note on `failure` on https://en.cppreference.com/w/cpp/atomic/atomic/compare_exchange
> 
> Seems like a bug in LLVM IR to enforce this, would be good if you could fix LLVM IR and not copy that specific mistake :)
Thanks for the heads up and references. I can attempt another patch later to bring us up to the C++17 standard. It looks like we'll need to :
  - Update any LLVM IR validation (I haven't looked yet how LLVM IR gets validated, I'm only familiar with MLIR at the moment). 
  - Update any documentation (I got the current set of restrictions from https://llvm.org/docs/LangRef.html#cmpxchg-instruction.
  - Remove this restriction from the MLIR LLVM dialect



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72995