This is an archive of the discontinued LLVM Phabricator instance.

Do a sign-extension in a compare-and-swap of 32 bit in RV64A

Authored by fpallares on Mar 1 2019, 7:16 AM.



AtomicCmpSwapWithSuccess is legalised into an AtomicCmpSwap plus a comparison. This requires an extension of the value which, by default, is a zero-extension.

When we later lower AtomicCmpSwap into a PseudoCmpXchg32 and then expanded in RISCVExpandPseudoInsts.cpp, the lr.w instruction does a sign-extension.

This mismatch of extensions causes the comparison to fail when the compared value is negative.

This change overrides TargetLowering::getExtendForAtomicOps for RISC-V so it does a sign-extension instead.

Diff Detail


Event Timeline

fpallares created this revision.Mar 1 2019, 7:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 1 2019, 7:16 AM
asb added a comment.Mar 11 2019, 8:36 AM

Great catch - thank you!

Could you please modify the test to use (which on balance we find easier to maintain), and add a comment indicating the original motivation for the test.

Thank you. I've updated the test using '' and I've added a comment as suggested.

asb accepted this revision.Mar 11 2019, 2:35 PM

Looks good to me. Thanks again for catching this issue, and of course for submitting the fix!

This revision is now accepted and ready to land.Mar 11 2019, 2:35 PM
This revision was automatically updated to reflect the committed changes.

I'm glad it helps, thank you for your time!