On Fri, 28 Aug 2020 at 14:00, Richard Earnshaw
<richard.earns...@foss.arm.com> wrote:
>
> On 27/08/2020 14:27, Christophe Lyon via Gcc-patches wrote:
> > In comment 14 from PR94538, it was suggested to switch off jump tables
> > on thumb-1 cores when using -mpure-code, like we already do for thumb-2.
> >
> > This is what this patch does, and also restores the previous value of
> > CASE_VECTOR_PC_RELATIVE since it was not the right way of handling
> > this.
> >
> > It also adds a new test, since the existing no-casesi.c did not catch
> > this problem.
> >
> > Tested by running the whole testsuite with -mpure-code -mcpu=cortex-m0
> > -mfloat-abi=soft, no regression and the new test passes (and fails
> > without the fix).
> >
> > 2020-08-27 Christophe Lyon <christophe.l...@linaro.org>
> >
> > gcc/
> > * config/arm/arm.h (CASE_VECTOR_PC_RELATIVE): Remove condition on
> > -mpure-code.
> > * config/arm/thumb1.md (tablejump): Disable when -mpure-code.
> >
> > gcc/testsuite/
> > * gcc.target/arm/pure-code/pr96768.c: New test.
> > ---
> > gcc/config/arm/arm.h | 5 ++---
> > gcc/config/arm/thumb1.md | 2 +-
> > gcc/testsuite/gcc.target/arm/pure-code/pr96768.c | 21 +++++++++++++++++++++
> > 3 files changed, 24 insertions(+), 4 deletions(-)
> > create mode 100644 gcc/testsuite/gcc.target/arm/pure-code/pr96768.c
> >
> > diff --git a/gcc/config/arm/arm.h b/gcc/config/arm/arm.h
> > index 3887c51..7d43721 100644
> > --- a/gcc/config/arm/arm.h
> > +++ b/gcc/config/arm/arm.h
> > @@ -1975,10 +1975,9 @@ enum arm_auto_incmodes
> > for the index in the tablejump instruction. */
> > #define CASE_VECTOR_MODE Pmode
> >
> > -#define CASE_VECTOR_PC_RELATIVE ((TARGET_THUMB2
> > \
> > +#define CASE_VECTOR_PC_RELATIVE (TARGET_THUMB2
> > \
> > || (TARGET_THUMB1 \
> > - && (optimize_size || flag_pic))) \
> > - && (!target_pure_code))
> > + && (optimize_size || flag_pic)))
> >
> >
> > #define CASE_VECTOR_SHORTEN_MODE(min, max, body) \
> > diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md
> > index f0129db..602039e 100644
> > --- a/gcc/config/arm/thumb1.md
> > +++ b/gcc/config/arm/thumb1.md
> > @@ -2012,7 +2012,7 @@ (define_insn "*epilogue_insns"
> > (define_expand "tablejump"
> > [(parallel [(set (pc) (match_operand:SI 0 "register_operand"))
> > (use (label_ref (match_operand 1 "" "")))])]
> > - "TARGET_THUMB1"
> > + "TARGET_THUMB1 && !target_pure_code"
> > "
> > if (flag_pic)
> > {
> > diff --git a/gcc/testsuite/gcc.target/arm/pure-code/pr96768.c
> > b/gcc/testsuite/gcc.target/arm/pure-code/pr96768.c
> > new file mode 100644
> > index 0000000..fd4cad5
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.target/arm/pure-code/pr96768.c
> > @@ -0,0 +1,21 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-mpure-code" } */
> > +
> > +int f2 (int x, int y)
> > +{
> > + switch (x)
> > + {
> > + case 0: return y + 0;
> > + case 1: return y + 1;
> > + case 2: return y + 2;
> > + case 3: return y + 3;
> > + case 4: return y + 4;
> > + case 5: return y + 5;
> > + }
> > + return y;
> > +}
> > +
> > +/* We do not want any load from literal pool, but accept loads from r7
> > + (frame pointer, used at -O0). */
> > +/* { dg-final { scan-assembler-not "ldr\tr\\d+, \\\[r\[0-689\]+\\\]" } } */
> > +/* { dg-final { scan-assembler "text,\"0x20000006\"" } } */
> >
>
> Having switch tables is only a problem if they are embedded in the .text
> segment. But the case you show in the PR has them in the .rodata
> segment, so is this really necessary?
>
Well, in the original PR94538, comment #14, Wilco said this was the best option.
> R.
