Hi Richard:

Test with x86 and rl78, both are rejected by the front-end if modes
are different from Pmode/ptr_mode,
so I'm gonna commit this change :)

Testcase for x86 / x86_64:
```
void test(void)
{
 int __seg_fs *f = (int __seg_fs *)16;
 int __seg_fs *g = (int __seg_fs *)32;
 __builtin___clear_cache (f, g); // error: passing argument 1 of
‘__builtin___clear_cache’ from pointer to non-enclosed address space
}
```

Testcase for rl78 (Pmode == HImode):
```
void test(void)
{
 int __near *f = (int __near *)16;  // mode == HImode, same as Pmode
 int __near *g = (int __near *)32;
 __builtin___clear_cache (f, g);  // OK to compile
}

void test2(void)
{
 int __far *f = (int __far *)16; // mode == SImode
 int __far *g = (int __far *)32;
 __builtin___clear_cache (f, g);  // error: passing argument 1 of
‘__builtin___clear_cache’ from pointer to non-enclosed address space
}
```

On Fri, Oct 8, 2021 at 2:47 PM Richard Biener
<richard.guent...@gmail.com> wrote:
>
> 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
> >

Reply via email to