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

Mon Jan 8 09:55:32 PST 2018

nemanjai added a comment.

> 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.

I have a similar understanding of the hook - i.e. how the results of atomic operations are extended. I guess I just see ATOMIC_CMP_SWAP as one where the input should conform to this as well as it is really a load-compare-store operation, so its input will be compared to a value that is loaded atomically. Namely, it's **input** should have the same high bits as the **result** of ATOMIC_LOAD.

> 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.

If a target has sub-word instructions for all three operations, I suppose it doesn't care if the loaded value and the comparand have different high bits. In all other cases, the target ultimately has to do the work of ensuring the high bits are set the same way for both values at some point. It seemed to me that legalization would be a logical place for ensuring that work is done, but of course since there are objections, I'll just fix that in the PPC target.

As far as this patch is concerned (or https://reviews.llvm.org/D38215), I think it should still be committed since it truly appears to be the right thing to do. And for the one I originally posted, I'll abandon the approach of fixing this in legalization and I'll fix it in the PPC back end.

Repository:
  rL LLVM

https://reviews.llvm.org/D41798