public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH v2] [PR/target 100316] Allow constant address for __builtin___clear_cache.
@ 2021-10-07 16:45 Kito Cheng
  2021-10-08  6:43 ` Richard Biener
  0 siblings, 1 reply; 2+ messages in thread
From: Kito Cheng @ 2021-10-07 16:45 UTC (permalink / raw)
  To: gcc-patches, kito.cheng, jimw, christophm30, oliva, pinskia; +Cc: 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;
```

Changes v1 -> v2:
- Check is CONST_INT intead of cehck mode, no new testcase, since
  constant value with other type like CONST_DOUBLE will catched by
  front-end.
e.g.
Code:
```c
void foo(){
  __builtin___clear_cache(1.11, 0);
}
```
Error message:
```
clearcache-double.c: In function 'foo':
clearcache-double.c:2:27: error: incompatible type for argument 1 of '__builtin___clear_cache'
    2 |   __builtin___clear_cache(1.11, 0);
      |                           ^~~~
      |                           |
      |                           double
clearcache-double.c:2:27: note: expected 'void *' but argument is of type 'double'
```

gcc/ChangeLog:

	PR target/100316
	* builtins.c (maybe_emit_call_builtin___clear_cache): Allow
	CONST_INT 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..8c3ad302468 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
+       && !CONST_INT_P (begin))
+      || (GET_MODE (end) != ptr_mode && GET_MODE (end) != Pmode
+	  && !CONST_INT_P (end)))
     {
       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


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH v2] [PR/target 100316] Allow constant address for __builtin___clear_cache.
  2021-10-07 16:45 [PATCH v2] [PR/target 100316] Allow constant address for __builtin___clear_cache Kito Cheng
@ 2021-10-08  6:43 ` Richard Biener
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Biener @ 2021-10-08  6:43 UTC (permalink / raw)
  To: Kito Cheng
  Cc: GCC Patches, Kito Cheng, Jim Wilson, christophm30,
	Alexandre Oliva, Andrew Pinski

On Thu, Oct 7, 2021 at 6:45 PM 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;
> ```
>
> Changes v1 -> v2:
> - Check is CONST_INT intead of cehck mode, no new testcase, since
>   constant value with other type like CONST_DOUBLE will catched by
>   front-end.
> e.g.
> Code:
> ```c
> void foo(){
>   __builtin___clear_cache(1.11, 0);
> }
> ```
> Error message:
> ```
> clearcache-double.c: In function 'foo':
> clearcache-double.c:2:27: error: incompatible type for argument 1 of '__builtin___clear_cache'
>     2 |   __builtin___clear_cache(1.11, 0);
>       |                           ^~~~
>       |                           |
>       |                           double
> clearcache-double.c:2:27: note: expected 'void *' but argument is of type 'double'
> ```
>
> gcc/ChangeLog:
>
>         PR target/100316
>         * builtins.c (maybe_emit_call_builtin___clear_cache): Allow
>         CONST_INT 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..8c3ad302468 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
> +       && !CONST_INT_P (begin))
> +      || (GET_MODE (end) != ptr_mode && GET_MODE (end) != Pmode
> +         && !CONST_INT_P (end)))
>      {
>        error ("both arguments to %<__builtin___clear_cache%> must be pointers");
>        return;

I don't think it makes sense to emit user-facing diagnostics about
this from here,
user code calling this go through expand_builtin___clear_cache which performs
the check and diagnostic on the argument types which is more appropriate and
the code above is eventually called from special backend code directly but
that's nothing the user can do anything about.

So how about simply removing this check?

Thanks,
Richard.

> 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
>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-10-08  6:43 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-07 16:45 [PATCH v2] [PR/target 100316] Allow constant address for __builtin___clear_cache Kito Cheng
2021-10-08  6:43 ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).