sdesmalen marked 3 inline comments as done.
sdesmalen added inline comments.
================
Comment at: clang/include/clang/Basic/arm_sve.td:153
+}
+def ImmCheckPredicatePattern : ImmCheckType<0>; // 0..31
+def ImmCheck1_16 : ImmCheckType<1>; // 1..16
----------------
SjoerdMeijer wrote:
> would it make sense to call this `ImmCheck0_31`?
Sure, I can change that.
================
Comment at: clang/lib/CodeGen/CGBuiltin.cpp:7571
+ else if (Builtin->LLVMIntrinsic != 0) {
+ llvm::Type* OverloadedTy = getSVEType(TypeFlags);
+
----------------
efriedma wrote:
> I'm not sure why you need a second way to convert SVE types separate from
> ConvertType
For the intrinsics added here, that's indeed correct, but for others, the
OverloadedTy is not necessarily the return type. This is determined by the type
string in the arm_sve.td file. For example:
```def SVQDECH_S : SInst<"svqdech_pat[_{d}]", "ddIi", "s", MergeNone,
"aarch64_sve_sqdech",
^
```
The `default` type `d` (in this case both return value, and first parameter) is
the overloaded type.
For other intrinsics, such as
```def SVSHRNB : SInst<"svshrnb[_n_{d}]", "hdi", "silUsUiUl",
MergeNone, "aarch64_sve_shrnb",
^
```
The overloaded type is the first parameter, not the result type.
================
Comment at:
clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_qdech.c:10
+{
+ return svqdech_pat_s16(op, SV_VL8, 0); // expected-error-re {{argument value
{{[0-9]+}} is outside the valid range [1, 16]}}
+}
----------------
SjoerdMeijer wrote:
> nit: why use a regexp for the argument value and not just its value?
Mostly because for most tests, there is no ambiguity and it would be a hassle
to update all the generated tests. The regular expression is also used for many
similar tests in Clang for e.g. Neon or other builtins. I guess for these tests
it makes sense to be more explicit which operand is out of range (given that
there are two immediates). Are you happy for me to change it for these tests,
but not others where there is only a single immediate (i.e. where the message
cannot be ambiguous)?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D76678/new/
https://reviews.llvm.org/D76678
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
