[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 09:15:44 PST 2018


uweigand added a comment.

In https://reviews.llvm.org/D41798#969936, @nemanjai wrote:

> > Sure, the operands will have been any-extended from i8/i16 to i32.  But the target knows that this happened, because it knows that this is a i8/i16 ATOMIC_CMP_SWAP (via getMemoryVT).  In that case, if it actually requires a particular sign- or zero-extended version of the original i8/i16 constant, it can still do the appropriate in-reg extension from the any-extended i32 value it got.
>
> Oh OK, of course. I mentioned in the original patch that I can certainly fix this in the PPC back end. I imagine that other back ends have done the same thing one way or another (or have a bug in this they don't know about just as PPC does). However, I ultimately don't see the utility in type-legalization ignoring how the target wants these inputs extended and doing an any-extend when the target is going to have to pick one down the line. And the target has already stated how it wants atomics extended in that TLI hook.


But that's not what the TLI hook says as I understand it; the TLI hook simply allows the back-end to inform common code how the **result** of a sub-word ATOMIC_CMP_SWAP will have been extended (either by hardware or by the back-end specific implementation).  It doesn't say anything about inputs.

And it's not clear that inputs really **need** to be extended in any particular way anyway -- if the back-end has sub-word instructions, those will likely ignore high bits anyway; and if the back-end creates a compare-and-swap loop on the surrounding word, it may be able (like SystemZ) to implement the necessary comparisons without explicit extensions.  So IMO this is best left to each target.


Repository:
  rL LLVM

https://reviews.llvm.org/D41798





More information about the llvm-commits mailing list