[PATCH] [PR/target 100316] Allow constant address for __builtin___clear_cache.

[PATCH] [PR/target 100316] Allow constant address for __builtin___clear_cache.

[Kito Cheng]

__builtin___clear_cache was able to accept constant address for the
argument, but it seems no longer accept recently, and it even not
accept constant address which is hold in variable when optimization is
enable:

```
void foo3(){
  void *yy = (void*)0x1000;
  __builtin___clear_cache(yy, yy);
}
```

So this patch make BEGIN and END accept VOIDmode, like cselib_lookup_mem did per
Jim Wilson's suggestion.

```
static cselib_val *
cselib_lookup_mem (rtx x, int create)
{
  ...
  addr_mode = GET_MODE (XEXP (x, 0));
  if (addr_mode == VOIDmode)
    addr_mode = Pmode;
```

gcc/ChangeLog:

	PR target/100316
	* builtins.c (maybe_emit_call_builtin___clear_cache): Allow
	VOIDmode for begin and end.

gcc/testsuite/ChangeLog:

	PR target/100316
	* gcc.c-torture/compile/pr100316.c: New.
---
 gcc/builtins.c                                 |  6 ++++--
 gcc/testsuite/gcc.c-torture/compile/pr100316.c | 18 ++++++++++++++++++
 2 files changed, 22 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr100316.c

diff --git a/gcc/builtins.c b/gcc/builtins.c
index 3e57eb03af0..960f07121fc 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -5163,8 +5163,10 @@ default_emit_call_builtin___clear_cache (rtx begin, rtx end)
 void
 maybe_emit_call_builtin___clear_cache (rtx begin, rtx end)
 {
-  if ((GET_MODE (begin) != ptr_mode && GET_MODE (begin) != Pmode)
-      || (GET_MODE (end) != ptr_mode && GET_MODE (end) != Pmode))
+  if ((GET_MODE (begin) != ptr_mode && GET_MODE (begin) != Pmode
+       && GET_MODE (begin) != VOIDmode)
+      || (GET_MODE (end) != ptr_mode && GET_MODE (end) != Pmode
+	  && GET_MODE (begin) != VOIDmode))
     {
       error ("both arguments to %<__builtin___clear_cache%> must be pointers");
       return;
diff --git a/gcc/testsuite/gcc.c-torture/compile/pr100316.c b/gcc/testsuite/gcc.c-torture/compile/pr100316.c
new file mode 100644
index 00000000000..38eca86f49f
--- /dev/null
+++ b/gcc/testsuite/gcc.c-torture/compile/pr100316.c
@@ -0,0 +1,18 @@
+void foo(){
+  __builtin___clear_cache(0, 0);
+}
+
+void foo1(){
+  __builtin___clear_cache((void*)0, (void*)0);
+}
+
+void foo2(){
+  void *yy = 0;
+  __builtin___clear_cache(yy, yy);
+}
+
+void foo3(){
+  void *yy = (void*)0x1000;
+  __builtin___clear_cache(yy, yy);
+}
+
-- 
2.33.0

Re: [PATCH] [PR/target 100316] Allow constant address for __builtin___clear_cache.

[Andrew Pinski]

On Thu, Oct 7, 2021 at 2:09 AM Kito Cheng <kito.cheng@sifive.com> wrote:
>
> __builtin___clear_cache was able to accept constant address for the
> argument, but it seems no longer accept recently, and it even not
> accept constant address which is hold in variable when optimization is
> enable:
>
> ```
> void foo3(){
>   void *yy = (void*)0x1000;
>   __builtin___clear_cache(yy, yy);
> }
> ```
>
> So this patch make BEGIN and END accept VOIDmode, like cselib_lookup_mem did per
> Jim Wilson's suggestion.
>
> ```
> static cselib_val *
> cselib_lookup_mem (rtx x, int create)
> {
>   ...
>   addr_mode = GET_MODE (XEXP (x, 0));
>   if (addr_mode == VOIDmode)
>     addr_mode = Pmode;
> ```
>
> gcc/ChangeLog:
>
>         PR target/100316
>         * builtins.c (maybe_emit_call_builtin___clear_cache): Allow
>         VOIDmode for begin and end.
>
> gcc/testsuite/ChangeLog:
>
>         PR target/100316
>         * gcc.c-torture/compile/pr100316.c: New.
> ---
>  gcc/builtins.c                                 |  6 ++++--
>  gcc/testsuite/gcc.c-torture/compile/pr100316.c | 18 ++++++++++++++++++
>  2 files changed, 22 insertions(+), 2 deletions(-)
>  create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr100316.c
>
> diff --git a/gcc/builtins.c b/gcc/builtins.c
> index 3e57eb03af0..960f07121fc 100644
> --- a/gcc/builtins.c
> +++ b/gcc/builtins.c
> @@ -5163,8 +5163,10 @@ default_emit_call_builtin___clear_cache (rtx begin, rtx end)
>  void
>  maybe_emit_call_builtin___clear_cache (rtx begin, rtx end)
>  {
> -  if ((GET_MODE (begin) != ptr_mode && GET_MODE (begin) != Pmode)
> -      || (GET_MODE (end) != ptr_mode && GET_MODE (end) != Pmode))
> +  if ((GET_MODE (begin) != ptr_mode && GET_MODE (begin) != Pmode
> +       && GET_MODE (begin) != VOIDmode)
> +      || (GET_MODE (end) != ptr_mode && GET_MODE (end) != Pmode
> +         && GET_MODE (begin) != VOIDmode))
>      {
>        error ("both arguments to %<__builtin___clear_cache%> must be pointers");
>        return;

Seems like this error should be an gcc_assert at this point.  The
error message should have come from expand_builtin___clear_cache
already.
And the check for VOIDmode should really be a check for CONST_INT.

Thanks,
Andrew Pinski

> diff --git a/gcc/testsuite/gcc.c-torture/compile/pr100316.c b/gcc/testsuite/gcc.c-torture/compile/pr100316.c
> new file mode 100644
> index 00000000000..38eca86f49f
> --- /dev/null
> +++ b/gcc/testsuite/gcc.c-torture/compile/pr100316.c
> @@ -0,0 +1,18 @@
> +void foo(){
> +  __builtin___clear_cache(0, 0);
> +}
> +
> +void foo1(){
> +  __builtin___clear_cache((void*)0, (void*)0);
> +}
> +
> +void foo2(){
> +  void *yy = 0;
> +  __builtin___clear_cache(yy, yy);
> +}
> +
> +void foo3(){
> +  void *yy = (void*)0x1000;
> +  __builtin___clear_cache(yy, yy);
> +}
> +
> --
> 2.33.0
>

Re: [PATCH] [PR/target 100316] Allow constant address for __builtin___clear_cache.

[Kito Cheng]

Hi Andrew:

> Seems like this error should be an gcc_assert at this point.  The
> error message should have come from expand_builtin___clear_cache
> already.

Actually it will emit errors here, so put gcc_assert will cause ICE :p

> And the check for VOIDmode should really be a check for CONST_INT.

Yeah, that makes sense, thanks!

Re: [PATCH] [PR/target 100316] Allow constant address for __builtin___clear_cache.

[Andrew Pinski]

On Thu, Oct 7, 2021 at 9:47 AM Kito Cheng <kito.cheng@gmail.com> wrote:
>
> Hi Andrew:
>
> > Seems like this error should be an gcc_assert at this point.  The
> > error message should have come from expand_builtin___clear_cache
> > already.
>
> Actually it will emit errors here, so put gcc_assert will cause ICE :p

The error message would have been emitted from
expand_builtin___clear_cache and maybe_emit_call_builtin___clear_cache
 would not have been called from user code.
All other uses of maybe_emit_call_builtin___clear_cache are internal
to gcc and should have the correct mode so asserting is the right
thing to do.

Thanks,
Andrew Pinski

>
> > And the check for VOIDmode should really be a check for CONST_INT.
>
> Yeah, that makes sense, thanks!

Re: [PATCH] [PR/target 100316] Allow constant address for __builtin___clear_cache.

[Kito Cheng]

Hi Andrew:

> The error message would have been emitted from
> expand_builtin___clear_cache and maybe_emit_call_builtin___clear_cache
>  would not have been called from user code.
> All other uses of maybe_emit_call_builtin___clear_cache are internal
> to gcc and should have the correct mode so asserting is the right
> thing to do.

I got your point, and agree that, just like when we feed double types,
such type error should already catched by other place   :)