[PATCH] D103284: [AArch64][RISCV] Make sure isel correctly honors failure orderings.

[Eli Friedman via Phabricator via llvm-commits]

efriedma added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/MachineMemOperand.h:261
+  /// other than cmpxchg, this is equivalent to getOrdering().)
+  AtomicOrdering getMergedOrdering() const {
+    AtomicOrdering Ordering = getOrdering();
----------------
jyknight wrote:
> I think this is a confusing API now. The reason it made sense to have getOrdering return the success ordering for a cmpxchg before is because that was always stronger than the failure ordering. So, using the success order was a safe "default" ordering. That's no longer the case.
> 
> Maybe we should have getOrdering should return the merged ordering, and rename the existing getOrdering to getSuccessOrdering.
> 
> Or, actually I like this better: remove getOrdering entirely, by renaming to getSuccessOrdering. Then we'll have getSuccessOrdering, getFailureOrdering, and getMergedOrdering. That's much clearer.  (And, yes, this does still make sense for non-cmpxchg -- they always succeed, so the success ordering is the only relevant thing for them.)
So the idea is basically just rename getOrdering()->getSuccessOrdering()?  Sure, I'll look at doing that in a followup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103284