[PATCH] D41798: [LegalizeDAG] Fix ATOMIC_CMP_SWAP_WITH_SUCCESS legalization.

Ulrich Weigand via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 8 07:00:44 PST 2018


uweigand added a comment.

This patch seems to be identical to the one I proposed here:
https://reviews.llvm.org/D38215

(While as mentioned there, this is no longer needed on SystemZ, I still think this is a correct change.)

As to Nemanja's comments, in the default expansion of ATOMIC_CMP_SWAP_WITH_SUCCESS, the result of an ATOMIC_CMP_SWAP call is compared with the compare value.  Since the result of ATOMIC_CMP_SWAP may be differently extended depending on the platform (this is what TLI.getExtendForAtomicOps is all about), the compare value must be extended in the same way.  E.g. on Mips, where TLI.getExtendForAtomicOps is SIGN_EXTEND, the compare value must likewise be always sign-extended, or else the comparison will fail.

Given that which extension is correct depends on the platform, it doesn't seem to make much sense to me to do an unconditional zero-extension of the compare value in common code ahead of time.

In any case, I'm not sure why the *input* to ATOMIC_CMP_SWAP should be extended in any way in the first place: the back-end gets told the actual type explicitly, and if any extension is necessary due to the particular way a back-end implements the operation, the back-end can just do that extension itself.


Repository:
  rL LLVM

https://reviews.llvm.org/D41798





More information about the llvm-commits mailing list