This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Lower unsigned saturation to USAT
ClosedPublic

Authored by thebolt on Dec 18 2017, 4:49 AM.

Details

Summary

Implement lower of unsigned saturation on an interval [0, k] where k + 1 is a power of two using USAT instruction in a similar way to how [~k, k] is lowered using SSAT on ARM models that supports it.

Patch by Marten Svanfeldt

Diff Detail

Event Timeline

thebolt created this revision.Dec 18 2017, 4:49 AM
fhahn added a subscriber: fhahn.Dec 18 2017, 4:58 AM

Thanks for the patch! Please upload a diff with more context: http://llvm.org/docs/Phabricator.html#id3 as this makes it easier to have a look at the code around your changes.

lib/Target/ARM/ARMISelLowering.cpp
4286

left over debug output

4312

nit: unrelated whitespace change.

12951

nit: unrelated change

12957

nit: unrelated change

fhahn added inline comments.Dec 18 2017, 4:59 AM
lib/Target/ARM/ARMISelLowering.cpp
4302

No braces needed here I think

thebolt updated this revision to Diff 127343.Dec 18 2017, 5:40 AM

Updated based on review.
Removed extra white space changes, extra braces and left-in debug code.

thebolt marked 5 inline comments as done.Dec 18 2017, 12:36 PM

I've updated the diff based on the comments, also regenerated the diff with more context as per the instructions on llvm site.

Thanks, this looks good to me, with the test extension. A nice extension to the SSAT case! Adding a couple of additional Arm people, to have a quick look.

test/CodeGen/ARM/usat.ll
1

This instruction should be available in ARMv6+ as well. Could you extend the test to cover that as well?

thebolt updated this revision to Diff 127515.Dec 19 2017, 6:03 AM

Add tests for ARMv6 (non-thumb)

fhahn accepted this revision.Dec 19 2017, 6:49 AM

LGTM, thanks

This revision is now accepted and ready to land.Dec 19 2017, 6:49 AM

I don't have any commit rights so would be happy if someone else picks it up

fhahn edited the summary of this revision. (Show Details)Dec 20 2017, 2:38 AM
fhahn closed this revision.Dec 20 2017, 3:14 AM
fhahn added a comment.Dec 20 2017, 3:15 AM

Done! Thanks for the patch :)