This is an archive of the discontinued LLVM Phabricator instance.

[ARM][atomicrmw] Fix CMP_SWAP_32 expand assert
ClosedPublic

Authored by tmatheson on Aug 3 2021, 8:48 AM.

Details

Summary

This assert is intended to ensure that the high registers are not
selected when it is passed to one of the thumb UXT instructions. However
it was triggering even for 32 bit where no UXT instruction is emitted.

Fixes PR51313.

Diff Detail

Event Timeline

tmatheson created this revision.Aug 3 2021, 8:48 AM
tmatheson requested review of this revision.Aug 3 2021, 8:48 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 3 2021, 8:48 AM
tmatheson updated this revision to Diff 363760.Aug 3 2021, 8:54 AM

Remove commented lines

dmgreen added a subscriber: dmgreen.Aug 3 2021, 9:09 AM
dmgreen added inline comments.
llvm/lib/Target/ARM/ARMExpandPseudoInsts.cpp
1651

I think it needs brackets around the ||

llvm/test/CodeGen/ARM/cmpxchg.mir
1

This needs to be rerun, to generate the check lines. It is also probably best if both the llc run lines have FileCheck's to check the output, even if they might be different.

tmatheson updated this revision to Diff 363778.Aug 3 2021, 9:37 AM

Parenthesise condition and update_mir_test_checks

tmatheson added inline comments.Aug 3 2021, 9:40 AM
llvm/test/CodeGen/ARM/cmpxchg.mir
1

I'm not convinced it needs the extra FileCheck noise, when all we really want to check is that it doesn't crash.

dmgreen accepted this revision.Aug 4 2021, 6:20 AM

LGTM. Thanks

llvm/test/CodeGen/ARM/cmpxchg.mir
1

It makes them easier to maintain, and the extra check lines are useful to make sure it looks OK.

This revision is now accepted and ready to land.Aug 4 2021, 6:20 AM
This revision was automatically updated to reflect the committed changes.