On Fri, Oct 8, 2021 at 4:40 AM Kito Cheng <kito.ch...@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 v2 -> v3:
> - Use gcc_assert rather than error, maybe_emit_call_builtin___clear_cache is
> internal use only, and we already checked the type in other place.
>
> 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, and use gcc_assert rather than
> error.
>
> gcc/testsuite/ChangeLog:
>
> PR target/100316
> * gcc.c-torture/compile/pr100316.c: New.
> ---
> gcc/builtins.c | 10 ++++------
> gcc/testsuite/gcc.c-torture/compile/pr100316.c | 18 ++++++++++++++++++
> 2 files changed, 22 insertions(+), 6 deletions(-)
> create mode 100644 gcc/testsuite/gcc.c-torture/compile/pr100316.c
>
> diff --git a/gcc/builtins.c b/gcc/builtins.c
> index 3e57eb03af0..80a1bb191c6 100644
> --- a/gcc/builtins.c
> +++ b/gcc/builtins.c
> @@ -5163,12 +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))
> - {
> - error ("both arguments to %<__builtin___clear_cache%> must be
> pointers");
> - return;
> - }
> + gcc_assert ((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)));
OK I guess.
I'm not 100% sure we might not ICE here when using
__builtin_clear_cache on a pointer
with some other than the default address-space which might have a mode
that's not
ptr_mode or Pmode?
Thanks,
Richard.
> if (targetm.have_clear_cache ())
> {
> 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
>
