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?

R.

Reply via email to