This is an archive of the discontinued LLVM Phabricator instance.

[ARM] ACLE parallel arithmetic and DSP style multiplications
ClosedPublic

Authored by SjoerdMeijer on Dec 6 2017, 4:49 AM.

Details

Summary
This is a follow up of r302131, in which we forgot to add SemaChecking
tests. Adding these tests revealed two problems which have been fixed:
- added missing intrinsic __qdbl,
- properly range checking ssat16 and usat16.

Diff Detail

Repository
rL LLVM

Event Timeline

SjoerdMeijer created this revision.Dec 6 2017, 4:49 AM
samparker edited edge metadata.Dec 6 2017, 5:15 AM

Thanks for looking into this!

include/clang/Basic/BuiltinsARM.def
39 ↗(On Diff #125705)

Do we now need a codegen tests for this one?

test/Sema/builtins-arm.c
161 ↗(On Diff #125705)

Interesting that this doesn't give an error?

236 ↗(On Diff #125705)

Can / should we warn the user when using signed arguments? Looks like I missed the unsigned qualifiers in the original patch.

SjoerdMeijer added inline comments.Dec 6 2017, 6:10 AM
include/clang/Basic/BuiltinsARM.def
39 ↗(On Diff #125705)

It's there already.

test/Sema/builtins-arm.c
161 ↗(On Diff #125705)

No, was just playing a little bit with "6.3.1.4 Real floating and integer" conversions here: "When a finite value of real floating type is converted to an integer type other than_Bool, the fractional part is discarded ... "

236 ↗(On Diff #125705)

Similarly, was playing a little bit with implicit conversions here: -1 is converted to unsigned. So, it's ok.

samparker accepted this revision.Dec 6 2017, 8:16 AM

Great, LGTM, many thanks for doing this!

This revision is now accepted and ready to land.Dec 6 2017, 8:16 AM
This revision was automatically updated to reflect the committed changes.
This revision was automatically updated to reflect the committed changes.