[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

[Sander de Smalen via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG515020c091e7: [SveEmitter] Add more immediate operand 
checks. (authored by sdesmalen).

Changed prior to commit:
  https://reviews.llvm.org/D76679?vs=252246=258724#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76679

Files:
  clang/include/clang/Basic/arm_sve.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_asrd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ext.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_tmad.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_asrd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_tmad.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_qshlu.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_shrnb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_qshlu.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_shrnb.c
  clang/utils/TableGen/SveEmitter.cpp

Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -456,6 +456,9 @@
 Bitwidth = ElementBitwidth;
 NumVectors = 0;
 break;
+  case 'h':
+ElementBitwidth /= 2;
+break;
   case 'P':
 Signed = true;
 Float = false;
@@ -463,6 +466,11 @@
 Bitwidth = 16;
 ElementBitwidth = 1;
 break;
+  case 'u':
+Predicate = false;
+Signed = false;
+Float = false;
+break;
   case 'i':
 Predicate = false;
 Float = false;
Index: clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_shrnb.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_shrnb.c
@@ -0,0 +1,47 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE2 -triple aarch64-none-linux-gnu -target-feature +sve2 -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE2 -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve2 -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+
+#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
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+#include 
+
+svint8_t test_svshrnb_n_s16(svint16_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 8]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_s16,,)(op1, 0);
+}
+
+svint16_t test_svshrnb_n_s32(svint32_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 16]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_s32,,)(op1, 0);
+}
+
+svint32_t test_svshrnb_n_s64(svint64_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 32]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_s64,,)(op1, 0);
+}
+
+svuint8_t test_svshrnb_n_u16(svuint16_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 8]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_u16,,)(op1, 0);
+}
+
+svuint16_t test_svshrnb_n_u32(svuint32_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 16]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_u32,,)(op1, 0);
+}
+
+svuint32_t test_svshrnb_n_u64(svuint64_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [1, 32]}}
+  return SVE_ACLE_FUNC(svshrnb,_n_u64,,)(op1, 0);
+}
Index: clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_qshlu.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_qshlu.c
@@ -0,0 +1,35 @@
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE2 -triple aarch64-none-linux-gnu -target-feature +sve2 -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+// RUN: %clang_cc1 -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE2 -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu -target-feature +sve2 -fallow-half-arguments-and-returns -fsyntax-only -verify %s
+
+#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
+#else
+#define SVE_ACLE_FUNC(A1,A2,A3,A4) A1##A2##A3##A4
+#endif
+
+#include 
+
+svuint8_t test_svqshlu_n_s8_m(svbool_t pg, svint8_t op1)
+{
+  // expected-error-re@+1 {{argument value {{[0-9]+}} is outside the valid range [0, 7]}}
+  return SVE_ACLE_FUNC(svqshlu,_n_s8,_m,)(pg, op1, -1);
+}
+
+svuint16_t test_svqshlu_n_s16_m(svbool_t pg, svint16_t 

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

In D76679#1957399 , @SjoerdMeijer 
wrote:

> I think the float16 discussion is an interesting one, but doesn't necessarily 
> need to be done here. I am asking some questions offline, but if we ever come 
> to a different opinion on it, then we can follow up so it's somewhat 
> orthogonal to this change, and so this looks fine to me.


Thanks for reviewing the patch Sjoerd!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76679



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

[Sjoerd Meijer via Phabricator via cfe-commits]

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

I think the float16 discussion is an interesting one, but doesn't necessarily 
need to be done here. I am asking some questions offline, but if we ever come 
to a different opinion on it, then we can follow up so it's somewhat orthogonal 
to this change, and so this looks fine to me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76679



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.

Just to clarify my last sentence:

> Now I am wondering why the ARM SVE ACLE is using float16_t, and not just 
> _Float16. Do you have any insights in that too perhaps?

What I meant to say is why SVE intrinsics are not using _Float16?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76679



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c:1
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-fallow-half-arguments-and-returns -fsyntax-only -verify -D__ARM_FEATURE_SVE %s
+

sdesmalen wrote:
> SjoerdMeijer wrote:
> > sdesmalen wrote:
> > > SjoerdMeijer wrote:
> > > > sdesmalen wrote:
> > > > > SjoerdMeijer wrote:
> > > > > > Just curious about the `-fallow-half-arguments-and-returns`, do you 
> > > > > > need that here?
> > > > > > 
> > > > > > And if not here, why do you need it elsewhere (looks enabled on all 
> > > > > > tests)?
> > > > > It's not needed for this test, but we've generated most of our tests 
> > > > > from the ACLE spec and the tests that use a scalar float16_t (== 
> > > > > __fp16) will need this, such as the ACLE intrinsic:
> > > > > 
> > > > >   svfloat16_t svadd_m(svbool_t, svfloat16_t, float16_t);
> > > > > 
> > > > > If you feel strongly about it, I could remove it from the other RUN 
> > > > > lines.
> > > > Well, I think this is my surprise then. Thinking out loud: we're 
> > > > talking SVE here, which always implies FP16. That's why I am surprised 
> > > > that we bother with a storage-type only type. Looking at the SVE ACLE I 
> > > > indeed see:
> > > > 
> > > >   float16_t equivalent to __fp16
> > > > 
> > > > where I was probably expecting:
> > > > 
> > > >   float16_t equivalent to _Float16
> > > > 
> > > > and with that everything would be sorted I guess, then we also don't 
> > > > need the hack^W workaround that is 
> > > > `-fallow-half-arguments-and-returns`. But maybe there is a good reason 
> > > > to use/choose `__fp16` that I don't see here. Probably worth a quick 
> > > > question for the ARM SVE ACLE, would you mind quickly checking?
> > > > 
> > > > 
> > > As just checked with @rsandifo-arm, the reason is that the definition of 
> > > `float16_t` has to be compatible with `arm_neon.h`, which uses `__fp16` 
> > > for both Clang and GCC.
> > I was suspecting it was compatability reasons, but perhaps not with 
> > `arm_neon.h`. So what exactly does it mean to be compatible with 
> > arm_neon.h? I mean, put simply and naively, if you target SVE, you include 
> > arm_sve.h, and go from there. How does that interact with arm_neon.h and 
> > why can float16_t not be a proper half type? 
> If you target SVE, you can still use Neon instructions, so it's still 
> possible to include arm_neon.h as well. If those have differing definitions 
> of float16_t, that may give trouble when using builtins from both the Neon 
> and SVE header files.
ok, thank, got it. So we are supporting this case:

   #include 
   #include 
   void foo () {
  neon_intrinsic();
  sve_intrinsic();
   }

Well, I find this very unfortunate, because it could have been so beautiful, 
and now we're still have this storage-only type while even the ACLE discourages 
its use. The use of `-fallow-half-arguments-and-returns` is just a minor 
annoyance, but point is it shouldn't have been necessary.

Now I am wondering why the ARM SVE ACLE  is using float16_t, and not just 
_Float16. Do you  have any insights in that too perhaps? 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76679



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen marked an inline comment as done.
sdesmalen added inline comments.



Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c:1
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-fallow-half-arguments-and-returns -fsyntax-only -verify -D__ARM_FEATURE_SVE %s
+

SjoerdMeijer wrote:
> sdesmalen wrote:
> > SjoerdMeijer wrote:
> > > sdesmalen wrote:
> > > > SjoerdMeijer wrote:
> > > > > Just curious about the `-fallow-half-arguments-and-returns`, do you 
> > > > > need that here?
> > > > > 
> > > > > And if not here, why do you need it elsewhere (looks enabled on all 
> > > > > tests)?
> > > > It's not needed for this test, but we've generated most of our tests 
> > > > from the ACLE spec and the tests that use a scalar float16_t (== 
> > > > __fp16) will need this, such as the ACLE intrinsic:
> > > > 
> > > >   svfloat16_t svadd_m(svbool_t, svfloat16_t, float16_t);
> > > > 
> > > > If you feel strongly about it, I could remove it from the other RUN 
> > > > lines.
> > > Well, I think this is my surprise then. Thinking out loud: we're talking 
> > > SVE here, which always implies FP16. That's why I am surprised that we 
> > > bother with a storage-type only type. Looking at the SVE ACLE I indeed 
> > > see:
> > > 
> > >   float16_t equivalent to __fp16
> > > 
> > > where I was probably expecting:
> > > 
> > >   float16_t equivalent to _Float16
> > > 
> > > and with that everything would be sorted I guess, then we also don't need 
> > > the hack^W workaround that is `-fallow-half-arguments-and-returns`. But 
> > > maybe there is a good reason to use/choose `__fp16` that I don't see 
> > > here. Probably worth a quick question for the ARM SVE ACLE, would you 
> > > mind quickly checking?
> > > 
> > > 
> > As just checked with @rsandifo-arm, the reason is that the definition of 
> > `float16_t` has to be compatible with `arm_neon.h`, which uses `__fp16` for 
> > both Clang and GCC.
> I was suspecting it was compatability reasons, but perhaps not with 
> `arm_neon.h`. So what exactly does it mean to be compatible with arm_neon.h? 
> I mean, put simply and naively, if you target SVE, you include arm_sve.h, and 
> go from there. How does that interact with arm_neon.h and why can float16_t 
> not be a proper half type? 
If you target SVE, you can still use Neon instructions, so it's still possible 
to include arm_neon.h as well. If those have differing definitions of 
float16_t, that may give trouble when using builtins from both the Neon and SVE 
header files.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76679



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c:1
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-fallow-half-arguments-and-returns -fsyntax-only -verify -D__ARM_FEATURE_SVE %s
+

sdesmalen wrote:
> SjoerdMeijer wrote:
> > sdesmalen wrote:
> > > SjoerdMeijer wrote:
> > > > Just curious about the `-fallow-half-arguments-and-returns`, do you 
> > > > need that here?
> > > > 
> > > > And if not here, why do you need it elsewhere (looks enabled on all 
> > > > tests)?
> > > It's not needed for this test, but we've generated most of our tests from 
> > > the ACLE spec and the tests that use a scalar float16_t (== __fp16) will 
> > > need this, such as the ACLE intrinsic:
> > > 
> > >   svfloat16_t svadd_m(svbool_t, svfloat16_t, float16_t);
> > > 
> > > If you feel strongly about it, I could remove it from the other RUN lines.
> > Well, I think this is my surprise then. Thinking out loud: we're talking 
> > SVE here, which always implies FP16. That's why I am surprised that we 
> > bother with a storage-type only type. Looking at the SVE ACLE I indeed see:
> > 
> >   float16_t equivalent to __fp16
> > 
> > where I was probably expecting:
> > 
> >   float16_t equivalent to _Float16
> > 
> > and with that everything would be sorted I guess, then we also don't need 
> > the hack^W workaround that is `-fallow-half-arguments-and-returns`. But 
> > maybe there is a good reason to use/choose `__fp16` that I don't see here. 
> > Probably worth a quick question for the ARM SVE ACLE, would you mind 
> > quickly checking?
> > 
> > 
> As just checked with @rsandifo-arm, the reason is that the definition of 
> `float16_t` has to be compatible with `arm_neon.h`, which uses `__fp16` for 
> both Clang and GCC.
I was suspecting it was compatability reasons, but perhaps not with 
`arm_neon.h`. So what exactly does it mean to be compatible with arm_neon.h? I 
mean, put simply and naively, if you target SVE, you include arm_sve.h, and go 
from there. How does that interact with arm_neon.h and why can float16_t not be 
a proper half type? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76679



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen marked an inline comment as done.
sdesmalen added a subscriber: rsandifo-arm.
sdesmalen added inline comments.



Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c:1
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-fallow-half-arguments-and-returns -fsyntax-only -verify -D__ARM_FEATURE_SVE %s
+

SjoerdMeijer wrote:
> sdesmalen wrote:
> > SjoerdMeijer wrote:
> > > Just curious about the `-fallow-half-arguments-and-returns`, do you need 
> > > that here?
> > > 
> > > And if not here, why do you need it elsewhere (looks enabled on all 
> > > tests)?
> > It's not needed for this test, but we've generated most of our tests from 
> > the ACLE spec and the tests that use a scalar float16_t (== __fp16) will 
> > need this, such as the ACLE intrinsic:
> > 
> >   svfloat16_t svadd_m(svbool_t, svfloat16_t, float16_t);
> > 
> > If you feel strongly about it, I could remove it from the other RUN lines.
> Well, I think this is my surprise then. Thinking out loud: we're talking SVE 
> here, which always implies FP16. That's why I am surprised that we bother 
> with a storage-type only type. Looking at the SVE ACLE I indeed see:
> 
>   float16_t equivalent to __fp16
> 
> where I was probably expecting:
> 
>   float16_t equivalent to _Float16
> 
> and with that everything would be sorted I guess, then we also don't need the 
> hack^W workaround that is `-fallow-half-arguments-and-returns`. But maybe 
> there is a good reason to use/choose `__fp16` that I don't see here. Probably 
> worth a quick question for the ARM SVE ACLE, would you mind quickly checking?
> 
> 
As just checked with @rsandifo-arm, the reason is that the definition of 
`float16_t` has to be compatible with `arm_neon.h`, which uses `__fp16` for 
both Clang and GCC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76679



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added a comment.






Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c:1
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-fallow-half-arguments-and-returns -fsyntax-only -verify -D__ARM_FEATURE_SVE %s
+

sdesmalen wrote:
> SjoerdMeijer wrote:
> > Just curious about the `-fallow-half-arguments-and-returns`, do you need 
> > that here?
> > 
> > And if not here, why do you need it elsewhere (looks enabled on all tests)?
> It's not needed for this test, but we've generated most of our tests from the 
> ACLE spec and the tests that use a scalar float16_t (== __fp16) will need 
> this, such as the ACLE intrinsic:
> 
>   svfloat16_t svadd_m(svbool_t, svfloat16_t, float16_t);
> 
> If you feel strongly about it, I could remove it from the other RUN lines.
Well, I think this is my surprise then. Thinking out loud: we're talking SVE 
here, which always implies FP16. That's why I am surprised that we bother with 
a storage-type only type. Looking at the SVE ACLE I indeed see:

  float16_t equivalent to __fp16

where I was probably expecting:

  float16_t equivalent to _Float16

and with that everything would be sorted I guess, then we also don't need the 
hack^W workaround that is `-fallow-half-arguments-and-returns`. But maybe there 
is a good reason to use/choose `__fp16` that I don't see here. Probably worth a 
quick question for the ARM SVE ACLE, would you mind quickly checking?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76679



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen marked an inline comment as done.
sdesmalen added a comment.

(sorry, I wrote the comments earlier but forgot to click 'submit' :) )




Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c:1
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-fallow-half-arguments-and-returns -fsyntax-only -verify -D__ARM_FEATURE_SVE %s
+

SjoerdMeijer wrote:
> Just curious about the `-fallow-half-arguments-and-returns`, do you need that 
> here?
> 
> And if not here, why do you need it elsewhere (looks enabled on all tests)?
It's not needed for this test, but we've generated most of our tests from the 
ACLE spec and the tests that use a scalar float16_t (== __fp16) will need this, 
such as the ACLE intrinsic:

  svfloat16_t svadd_m(svbool_t, svfloat16_t, float16_t);

If you feel strongly about it, I could remove it from the other RUN lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76679



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

[Sjoerd Meijer via Phabricator via cfe-commits]

SjoerdMeijer added inline comments.



Comment at: clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c:1
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-fallow-half-arguments-and-returns -fsyntax-only -verify -D__ARM_FEATURE_SVE %s
+

Just curious about the `-fallow-half-arguments-and-returns`, do you need that 
here?

And if not here, why do you need it elsewhere (looks enabled on all tests)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76679



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D76679: [SveEmitter] Add more immediate operand checks.

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen created this revision.
sdesmalen added reviewers: efriedma, SjoerdMeijer, rovka.
Herald added a subscriber: tschuett.
Herald added a reviewer: rengolin.
Herald added a project: clang.
sdesmalen added a parent revision: D76678: [SveEmitter] Add range checks for 
immediates and predicate patterns..
sdesmalen added a child revision: D76680: [SveEmitter] Add immediate checks for 
lanes and complex imms.

This patch adds a number of intrinsics that take immediates with
varying ranges based on the element size one of the operands.

  svext:   immediate ranging 0 to (2048/sizeinbits(elt) - 1)
  svasrd:  immediate ranging 1..sizeinbits(elt)
  svqshlu: immediate ranging 1..sizeinbits(elt)/2
  ftmad:   immediate ranging 0..(sizeinbits(elt) - 1)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D76679

Files:
  clang/include/clang/Basic/arm_sve.td
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_asrd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_asrd_shortform.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ext.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_ext_shortform.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_tmad.c
  clang/test/CodeGen/aarch64-sve-intrinsics/acle_sve_tmad_shortform.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_asrd.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_ext.c
  clang/test/CodeGen/aarch64-sve-intrinsics/negative/acle_sve_tmad.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_qshlu.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_qshlu_shortform.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_shrnb.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/acle_sve2_shrnb_shortform.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_qshlu.c
  clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_shrnb.c
  clang/utils/TableGen/SveEmitter.cpp

Index: clang/utils/TableGen/SveEmitter.cpp
===
--- clang/utils/TableGen/SveEmitter.cpp
+++ clang/utils/TableGen/SveEmitter.cpp
@@ -471,6 +471,9 @@
 Bitwidth = ElementBitwidth;
 NumVectors = 0;
 break;
+  case 'h':
+ElementBitwidth /= 2;
+break;
   case 'P':
 Signed = true;
 Float = false;
@@ -478,6 +481,11 @@
 Bitwidth = 16;
 ElementBitwidth = 1;
 break;
+  case 'u':
+Predicate = false;
+Signed = false;
+Float = false;
+break;
   case 'i':
 Predicate = false;
 Float = false;
Index: clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_shrnb.c
===
--- /dev/null
+++ clang/test/CodeGen/aarch64-sve2-intrinsics/negative/acle_sve2_shrnb.c
@@ -0,0 +1,65 @@
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve -fallow-half-arguments-and-returns -fsyntax-only -verify -D__ARM_FEATURE_SVE -D__ARM_FEATURE_SVE2 %s
+
+
+#include 
+
+svint8_t test_svshrnb_n_s16(svint16_t op1)
+{
+  return svshrnb_n_s16(op1, 0); // expected-error-re {{argument value {{[0-9]+}} is outside the valid range [1, 8]}}
+}
+
+svint8_t test_svshrnb(svint16_t op1)
+{
+  return svshrnb(op1, 0); // expected-error-re {{argument value {{[0-9]+}} is outside the valid range [1, 8]}}
+}
+
+svint16_t test_svshrnb_n_s32(svint32_t op1)
+{
+  return svshrnb_n_s32(op1, 0); // expected-error-re {{argument value {{[0-9]+}} is outside the valid range [1, 16]}}
+}
+
+svint16_t test_svshrnb_1(svint32_t op1)
+{
+  return svshrnb(op1, 0); // expected-error-re {{argument value {{[0-9]+}} is outside the valid range [1, 16]}}
+}
+
+svint32_t test_svshrnb_n_s64(svint64_t op1)
+{
+  return svshrnb_n_s64(op1, 0); // expected-error-re {{argument value {{[0-9]+}} is outside the valid range [1, 32]}}
+}
+
+svint32_t test_svshrnb_2(svint64_t op1)
+{
+  return svshrnb(op1, 0); // expected-error-re {{argument value {{[0-9]+}} is outside the valid range [1, 32]}}
+}
+
+svuint8_t test_svshrnb_n_u16(svuint16_t op1)
+{
+  return svshrnb_n_u16(op1, 0); // expected-error-re {{argument value {{[0-9]+}} is outside the valid range [1, 8]}}
+}
+
+svuint8_t test_svshrnb_3(svuint16_t op1)
+{
+  return svshrnb(op1, 0); // expected-error-re {{argument value {{[0-9]+}} is outside the valid range [1, 8]}}
+}
+
+svuint16_t test_svshrnb_n_u32(svuint32_t op1)
+{
+  return svshrnb_n_u32(op1, 0); // expected-error-re {{argument value {{[0-9]+}} is outside the valid range [1, 16]}}
+}
+
+svuint16_t test_svshrnb_4(svuint32_t op1)
+{
+  return svshrnb(op1, 0); // expected-error-re {{argument value {{[0-9]+}} is outside the valid range [1, 16]}}
+}
+
+svuint32_t test_svshrnb_n_u64(svuint64_t op1)
+{
+  return svshrnb_n_u64(op1, 0); // expected-error-re {{argument value {{[0-9]+}} is outside the valid range [1, 32]}}
+}
+
+svuint32_t test_svshrnb_5(svuint64_t op1)
+{
+  return svshrnb(op1, 0); // expected-error-re {{argument value