On Fri, Nov 28, 2025 at 2:53 PM Yuriy Kolerov <[email protected]> wrote: > > From: Michiel Derhaeg <[email protected]> > > This prevents linker errors when referencing small data using large > offsets. In practice it is very unlikely to be a real problem but this > was fixed because other compilers do this check and it ensures the > following tests now succeed: > - gcc.dg/torture/pr60115.c > - gcc.dg/torture/pr105665.c > > PR target/115650
I don't think it is a good practice to allow such large offsets (i.e., scaled) for small data. For one or two large arrays, this may work. However, having more than that will lead to linker errors. Basically, your proposed solution doesn't address the core issue. In my opinion, there are a number of solutions here: - sdata disabled by default in the gcc driver, and enable it on the user request. - implement a number of optimizations, including linker related that will convert the loads/store of sdata to regular access. Of course, the conversion of stores is a bit tricky. - drop the sdata idea, and use pc-relative + anchors, and use gp reg for that. Anyhow, I'll allow this patch to go, however, you need to address my comments before that. Cheers, Claudiu > > gcc/ChangeLog: > > * config/arc/arc.cc (legitimate_small_data_address_p): > Check offset size. > > gcc/testsuite/ChangeLog: > > * gcc.target/arc/sdata-6.c: New test. > > Signed-off-by: Michiel Derhaeg <[email protected]> > --- > gcc/config/arc/arc.cc | 24 ++++++++++++++++-------- > gcc/testsuite/gcc.target/arc/sdata-6.c | 17 +++++++++++++++++ > 2 files changed, 33 insertions(+), 8 deletions(-) > create mode 100644 gcc/testsuite/gcc.target/arc/sdata-6.c > > diff --git a/gcc/config/arc/arc.cc b/gcc/config/arc/arc.cc > index 145bc058b09..740b297142b 100644 > --- a/gcc/config/arc/arc.cc > +++ b/gcc/config/arc/arc.cc > @@ -386,27 +386,35 @@ legitimate_small_data_address_p (rtx x, machine_mode > mode) > return SYMBOL_REF_SMALL_P (x); > case PLUS: > { > - bool p0 = (GET_CODE (XEXP (x, 0)) == SYMBOL_REF) > - && SYMBOL_REF_SMALL_P (XEXP (x, 0)); > + if ((GET_CODE (XEXP (x, 0)) != SYMBOL_REF) > + || !SYMBOL_REF_SMALL_P (XEXP (x, 0))) > + return false; > > /* If no constant then we cannot do small data. */ > if (!CONST_INT_P (XEXP (x, 1))) > return false; > > - /* Small data relocs works with scalled addresses, check if > + const int offset = INTVAL (XEXP (x, 1)); > + int size = GET_MODE_SIZE (mode); > + size = size == 8 ? 4 : size; > + > + /* Small data relocs works with scaled addresses, check if > the immediate fits the requirements. */ > - switch (GET_MODE_SIZE (mode)) > + switch (size) > { > case 1: > - return p0; > + break; > case 2: > - return p0 && ((INTVAL (XEXP (x, 1)) & 0x1) == 0); > + if ((offset & 0x1) == 0) break; else return false; Not GNU standard. > case 4: > - case 8: > - return p0 && ((INTVAL (XEXP (x, 1)) & 0x3) == 0); > + if ((offset & 0x3) == 0) break; else return false; Likewise. > default: > return false; > } > + > + /* Reloc allows scaled signed 9 bits. */ > + const int v = (offset / size) >> 8; > + return v == 0 || v == -1; > } > default: > return false; > diff --git a/gcc/testsuite/gcc.target/arc/sdata-6.c > b/gcc/testsuite/gcc.target/arc/sdata-6.c > new file mode 100644 > index 00000000000..9b58ac54cbe > --- /dev/null > +++ b/gcc/testsuite/gcc.target/arc/sdata-6.c > @@ -0,0 +1,17 @@ > +/* { dg-do compile } */ > +/* { dg-options "-O2 -msdata" } */ > + > +__attribute__((section(".sdata"))) int a[300]; > + > +int f (void) > +{ > + return a[255]; > +} > + > +int g (void) > +{ > + return a[256]; > +} > + > +/* { dg-final { scan-assembler "ld_s\\s+r0,\\\[gp,@a@sda\\+1020\\\]" } } */ > +/* { dg-final { scan-assembler "ld\\s+r0,\\\[@a\\+1024\\\]" } } */ I am expecting a load operation using the as flag (i.e., ld.as ), the best will be to have a test checking the scaling for (8)/16/32 and 64 loads. > -- > 2.34.1 >
