Page MenuHomePhabricator

[ARM] Do not emit ldrexd/strexd on Cortex-M chips

Authored by aykevl on Feb 2 2021, 12:14 PM.



The ldrexd/strexd instructions are not supported on M-class chips, see for example which says:

All these 32-bit Thumb instructions are available in ARMv6T2 and above, except that LDREXD and STREXD are not available in the ARMv7-M architecture.

Looking at the ARMv8-M architecture, it appears that these instructions aren't supported either. The Architecture Reference Manual lists ldrex/strex but not ldrexd/strexd:

Compiler Explorer example on LLVM 11.0.0, which incorrectly emits ldrexd/strexd instructions:

Diff Detail

Event Timeline

aykevl created this revision.Feb 2 2021, 12:14 PM
aykevl requested review of this revision.Feb 2 2021, 12:14 PM
aykevl edited the summary of this revision. (Show Details)Feb 2 2021, 12:17 PM
aykevl edited the summary of this revision. (Show Details)
SjoerdMeijer added inline comments.Feb 3 2021, 6:07 AM

I think this could benefit from a comment saying that this is a special case for LDREXD and STREXD as they are not available in v7-m and v8-m.


Can you add an armv8m RUN line too?

aykevl updated this revision to Diff 321151.Feb 3 2021, 10:30 AM
  • added comment to shouldExpandAtomicCmpXchgInIR explaining Cortex-M exception
  • added checks for armv8m architecture (which are the same as armv7m)

Thanks for the review! I've updated the patch accordingly.


I've added a comment. The situation is really the same as for shouldExpandAtomicRMWInIR etc.


Sure, done. As far as I can see, it's the same for armv7m as for armv8m: they both only support 32-bit (4 byte) atomic operations. Not entirely sure about the check prefix but it's for the Cortex-M profile and others are similarly short.

SjoerdMeijer accepted this revision.Feb 4 2021, 2:16 AM

Cheers, LGTM.

This revision is now accepted and ready to land.Feb 4 2021, 2:16 AM
This revision was landed with ongoing or failed builds.Feb 4 2021, 12:56 PM
This revision was automatically updated to reflect the committed changes.

@SjoerdMeijer thank you for the quick review!