[PATCH] D41791: [DAG Legalization] Zero-extend the value compared in cmpxchg

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai abandoned this revision.
nemanjai added a comment.

Abandoning this as at least one target prefers that the compare input be any-extended and each target decide what to do about the high bits that may differ between the two values.



================
Comment at: lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:236
 
-  SDValue Op2 = GetPromotedInteger(N->getOperand(2));
+  SDValue Op2 = ZExtPromotedInteger(N->getOperand(2));
   SDValue Op3 = GetPromotedInteger(N->getOperand(3));
----------------
nemanjai wrote:
> efriedma wrote:
> > The value operand to a store is implicitly truncated.  The same thing happens for ATOMIC_LOAD_*.  The same thing happens for the third operand of ATOMIC_CMP_SWAP.  I think it's more confusing than helpful to add a special case for the compare operand of a compare.  Atomic compare-and-swap doesn't zero-extend on all targets, anyway; see getExtendForAtomicOps().
> I agree that this seems a bit confusing - i.e. why is one operand zero-extended whereas the other operand is essentially "any-extended". However, I think that the "store" operand of the atomic compare and exchange should be extended according to `getExtendForAtomicOps()` whereas the "compare" operand should always be zero-extended.
> The reason I think this is that the compare operand is essentially used for equality comparison similarly to SETEQ/SETNE for SETCC, whereas the other operand is used to load and store the value - so it should be handled the way a target desires to handle atomics.
I had planned to guard this zero-extension with `TLI.getExtendForAtomicOps() == ISD::ZERO_EXTEND`. However, since there are objections to doing this in legalization (see D41798), I'll abandon this and fix it in the PPC back end.


Repository:
  rL LLVM

https://reviews.llvm.org/D41791