Committed to trunk :)
On Mon, Oct 11, 2021 at 11:23 AM Kito Cheng <kito.ch...@sifive.com> wrote:
>
> 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
> > >
