This is an archive of the discontinued LLVM Phabricator instance.

[SveEmitter] IsInsertOp1SVALL and builtins for svqdec[bhwd] and svqinc[bhwd]
ClosedPublic

Authored by sdesmalen on Apr 17 2020, 1:47 PM.

Details

Summary

Some ACLE builtins leave out the argument to specify the predicate
pattern, which is expected to be expanded to an SV_ALL pattern.

This patch adds the flag IsInsertOp1SVALL to insert SV_ALL as the
second operand.

Diff Detail

Event Timeline

sdesmalen created this revision.Apr 17 2020, 1:47 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 17 2020, 1:47 PM
Herald added a subscriber: tschuett. · View Herald Transcript
SjoerdMeijer accepted this revision.Apr 20 2020, 6:47 AM

Looks reasonable

clang/include/clang/Basic/arm_sve.td
858

nit: just wondering if all these defs should be all capitals.

clang/lib/CodeGen/CGBuiltin.cpp
7919

would this be the most appropriate place to add the useful sentence from the description:

"Some ACLE builtins leave out the argument to specify the predicate
pattern, which is expected to be expanded to an SV_ALL pattern."

because that's what 31 is, right?

clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_qdecb.c
8

nit: used,unused -> "used/unused", or "used, unused"

This revision is now accepted and ready to land.Apr 20 2020, 6:47 AM
sdesmalen updated this revision to Diff 259554.Apr 23 2020, 6:55 AM
sdesmalen marked 3 inline comments as done.
  • Changed UNSIGNED_BYTE etc from upper-case to CamelCase.
  • Added comment explaining need to expand SV_ALL
clang/include/clang/Basic/arm_sve.td
858

They don't and I can see how this can be confused with intrinsics. I've changed it!

clang/lib/CodeGen/CGBuiltin.cpp
7919

Yes, good suggestion! I've added the comment.

clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_qdecb.c
8

I'd rather not fix this for this patch, because it would need changing for all other test files as well.

This revision was automatically updated to reflect the committed changes.