Page MenuHomePhabricator

[AArch64][SVE] Fix missed immediate selection due to mishandling of signedness

Authored by bsmith on May 4 2021, 7:09 AM.



The complex selection pattern for add/sub shifted immediates is
incorrect in it's handling of incoming constant values, in that it
does not properly anticipate the values to be signed extended to

Co-authored-by: Graham Hunter <>

Diff Detail

Event Timeline

bsmith created this revision.May 4 2021, 7:09 AM
bsmith requested review of this revision.May 4 2021, 7:09 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 4 2021, 7:09 AM
efriedma added inline comments.May 4 2021, 11:19 AM

Does checking ((int32_t)ImmVal >= -32768) && ((int32_t)ImmVal <= 32512)) actually do anything? All 16-bit immediates should be in that range.

bsmith added inline comments.May 5 2021, 3:45 AM

It possibly doesn't, however the thing that makes me nervous here is that the incoming CNode value is an i32 so could in theory (if something has gone wrong?) have a value outside of that range. It might be best to do it like this if nothing else to prevent an issue further up the pipeline propagating into a codegen fault here?

david-arm added inline comments.May 5 2021, 3:55 AM

I guess you could possibly remove those checks and replace them with an assert?

Matt added a subscriber: Matt.May 5 2021, 4:59 AM
bsmith updated this revision to Diff 343005.May 5 2021, 5:04 AM
bsmith marked 2 inline comments as done.
  • Fix clang format issues
  • Move range check to be an assert since it should always be true
peterwaller-arm added inline comments.May 6 2021, 3:20 AM

Suggestion: Use shift operation since this is about bit manipulation, and it's consistent with other uses nearby.

bsmith updated this revision to Diff 343418.May 6 2021, 8:27 AM
  • Use shift in place of divide for consistency.
peterwaller-arm accepted this revision.May 12 2021, 7:10 AM
This revision is now accepted and ready to land.May 12 2021, 7:10 AM