[PATCH] D78401: [SveEmitter] IsInsertOp1SVALL and builtins for svqdec[bhwd] and svqinc[bhwd]

Sjoerd Meijer via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 20 06:59:45 PDT 2020


SjoerdMeijer accepted this revision.
SjoerdMeijer added a comment.
This revision is now accepted and ready to land.

Looks reasonable



================
Comment at: clang/include/clang/Basic/arm_sve.td:530
+class sat_type<string u, string t> { string U = u; string T = t; }
+def SIGNED_BYTE         : sat_type<"",  "c">;
+def SIGNED_HALF         : sat_type<"",  "s">;
----------------
nit: just wondering if all these defs should be all capitals.


================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7684
 
+    if (TypeFlags.isInsertOp1SVALL())
+      Ops.insert(&Ops[1], Builder.getInt32(31));
----------------
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?


================
Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_qdecb.c:7
+#ifdef SVE_OVERLOADED_FORMS
+// A simple used,unused... macro, long enough to represent any SVE builtin.
+#define SVE_ACLE_FUNC(A1,A2_UNUSED,A3,A4_UNUSED) A1##A3
----------------
nit: used,unused -> "used/unused", or "used, unused"


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78401/new/

https://reviews.llvm.org/D78401





More information about the cfe-commits mailing list