ping?
On Mon, 28 Sep 2020 at 11:09, Christophe Lyon
<christophe.l...@linaro.org> wrote:
>
> With -mpure-code on v6m (thumb-1), to avoid a useless indirection when
> building the address of a symbol, we want to consider SYMBOL_REF as a
> legitimate constant. This way, we build the address using a series of
> upper/lower relocations instead of loading the address from memory.
>
> This patch also fixes a missing "clob" conds attribute for
> thumb1_movsi_insn, needed because that alternative clobbers the flags.
>
> 2020-09-28 Christophe Lyon <christophe.l...@linaro.org>
>
> gcc/
> * config/arm/arm.c (thumb_legitimate_constant_p): Add support for
> disabled literal pool in thumb-1.
> * config/arm/thumb1.md (thumb1_movsi_symbol_ref): Remove.
> (*thumb1_movsi_insn): Add support for SYMBOL_REF with -mpure-code.
>
> gcc/testsuite
> * gcc.target/arm/pure-code/pr96767.c: New test.
> ---
> gcc/config/arm/arm.c | 6 ++
> gcc/config/arm/thumb1.md | 102
> +++++++++++------------
> gcc/testsuite/gcc.target/arm/pure-code/pr96767.c | 10 +++
> 3 files changed, 63 insertions(+), 55 deletions(-)
> create mode 100644 gcc/testsuite/gcc.target/arm/pure-code/pr96767.c
>
> diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
> index 022ef6c..abe357e 100644
> --- a/gcc/config/arm/arm.c
> +++ b/gcc/config/arm/arm.c
> @@ -9485,6 +9485,12 @@ thumb_legitimate_constant_p (machine_mode mode
> ATTRIBUTE_UNUSED, rtx x)
> || CONST_DOUBLE_P (x)
> || CONSTANT_ADDRESS_P (x)
> || (TARGET_HAVE_MOVT && GET_CODE (x) == SYMBOL_REF)
> + /* On Thumb-1 without MOVT/MOVW and literal pool disabled,
> + we build the symbol address with upper/lower
> + relocations. */
> + || (TARGET_THUMB1
> + && GET_CODE (x) == SYMBOL_REF
> + && arm_disable_literal_pool)
> || flag_pic);
> }
>
> diff --git a/gcc/config/arm/thumb1.md b/gcc/config/arm/thumb1.md
> index 4a59d87..3dedcae 100644
> --- a/gcc/config/arm/thumb1.md
> +++ b/gcc/config/arm/thumb1.md
> @@ -43,27 +43,6 @@
>
>
>
> -(define_insn "thumb1_movsi_symbol_ref"
> - [(set (match_operand:SI 0 "register_operand" "=l")
> - (match_operand:SI 1 "general_operand" ""))
> - ]
> - "TARGET_THUMB1
> - && arm_disable_literal_pool
> - && GET_CODE (operands[1]) == SYMBOL_REF"
> - "*
> - output_asm_insn (\"movs\\t%0, #:upper8_15:%1\", operands);
> - output_asm_insn (\"lsls\\t%0, #8\", operands);
> - output_asm_insn (\"adds\\t%0, #:upper0_7:%1\", operands);
> - output_asm_insn (\"lsls\\t%0, #8\", operands);
> - output_asm_insn (\"adds\\t%0, #:lower8_15:%1\", operands);
> - output_asm_insn (\"lsls\\t%0, #8\", operands);
> - output_asm_insn (\"adds\\t%0, #:lower0_7:%1\", operands);
> - return \"\";
> - "
> - [(set_attr "length" "14")
> - (set_attr "conds" "clob")]
> -)
> -
> (define_insn "*thumb1_adddi3"
> [(set (match_operand:DI 0 "register_operand" "=l")
> (plus:DI (match_operand:DI 1 "register_operand" "%0")
> @@ -696,40 +675,53 @@ (define_insn "*thumb1_movsi_insn"
> case 7:
> /* pure-code alternative: build the constant byte by byte,
> instead of loading it from a constant pool. */
> - {
> - int i;
> - HOST_WIDE_INT op1 = INTVAL (operands[1]);
> - bool mov_done_p = false;
> - rtx ops[2];
> - ops[0] = operands[0];
> -
> - /* Emit upper 3 bytes if needed. */
> - for (i = 0; i < 3; i++)
> - {
> - int byte = (op1 >> (8 * (3 - i))) & 0xff;
> -
> - if (byte)
> - {
> - ops[1] = GEN_INT (byte);
> - if (mov_done_p)
> - output_asm_insn ("adds\t%0, %1", ops);
> - else
> - output_asm_insn ("movs\t%0, %1", ops);
> - mov_done_p = true;
> - }
> -
> - if (mov_done_p)
> - output_asm_insn ("lsls\t%0, #8", ops);
> - }
> + if (GET_CODE (operands[1]) == SYMBOL_REF)
> + {
> + output_asm_insn (\"movs\\t%0, #:upper8_15:%1\", operands);
> + output_asm_insn (\"lsls\\t%0, #8\", operands);
> + output_asm_insn (\"adds\\t%0, #:upper0_7:%1\", operands);
> + output_asm_insn (\"lsls\\t%0, #8\", operands);
> + output_asm_insn (\"adds\\t%0, #:lower8_15:%1\", operands);
> + output_asm_insn (\"lsls\\t%0, #8\", operands);
> + output_asm_insn (\"adds\\t%0, #:lower0_7:%1\", operands);
> + return \"\";
> + }
> + else
> + {
> + int i;
> + HOST_WIDE_INT op1 = INTVAL (operands[1]);
> + bool mov_done_p = false;
> + rtx ops[2];
> + ops[0] = operands[0];
> +
> + /* Emit upper 3 bytes if needed. */
> + for (i = 0; i < 3; i++)
> + {
> + int byte = (op1 >> (8 * (3 - i))) & 0xff;
> +
> + if (byte)
> + {
> + ops[1] = GEN_INT (byte);
> + if (mov_done_p)
> + output_asm_insn ("adds\t%0, %1", ops);
> + else
> + output_asm_insn ("movs\t%0, %1", ops);
> + mov_done_p = true;
> + }
> +
> + if (mov_done_p)
> + output_asm_insn ("lsls\t%0, #8", ops);
> + }
> +
> + /* Emit lower byte if needed. */
> + ops[1] = GEN_INT (op1 & 0xff);
> + if (!mov_done_p)
> + output_asm_insn ("movs\t%0, %1", ops);
> + else if (op1 & 0xff)
> + output_asm_insn ("adds\t%0, %1", ops);
> + return "";
> + }
>
> - /* Emit lower byte if needed. */
> - ops[1] = GEN_INT (op1 & 0xff);
> - if (!mov_done_p)
> - output_asm_insn ("movs\t%0, %1", ops);
> - else if (op1 & 0xff)
> - output_asm_insn ("adds\t%0, %1", ops);
> - return "";
> - }
> case 8: return "ldr\t%0, %1";
> case 9: return "str\t%1, %0";
> case 10: return "mov\t%0, %1";
> @@ -740,7 +732,7 @@ (define_insn "*thumb1_movsi_insn"
> (set_attr "pool_range" "*,*,*,*,*,*,*, *,1018,*,*")
> (set_attr "arch" "t1,t1,v8mb,t1,t1,t1,t1,t1,t1,t1,t1")
> (set_attr "required_for_purecode" "no,no,no,no,no,no,no,yes,no,no,no")
> - (set_attr "conds"
> "set,clob,nocond,*,*,nocond,nocond,nocond,nocond,nocond,nocond")])
> + (set_attr "conds"
> "set,clob,nocond,*,*,nocond,nocond,clob,nocond,nocond,nocond")])
>
> ; Split the load of 64-bit constant into two loads for high and low 32-bit
> parts respectively
> ; to see if we can load them in fewer instructions or fewer cycles.
> diff --git a/gcc/testsuite/gcc.target/arm/pure-code/pr96767.c
> b/gcc/testsuite/gcc.target/arm/pure-code/pr96767.c
> new file mode 100644
> index 0000000..cb3ee68
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/arm/pure-code/pr96767.c
> @@ -0,0 +1,10 @@
> +/* { dg-do compile } */
> +/* { dg-options "-mpure-code" } */
> +
> +int x;
> +int f1 (void) { return x; }
> +
> +/* We expect only one indirect load like ldr r3, [r3]. In some
> + configurations there is an additional ldr rX, [sp], #4 which is not
> + related to what we check here, so make sure not to match it. */
> +/* { dg-final { scan-assembler-times "ldr\tr\[0-9\]+, \\\[r\[0-9\]+\\\]" 1 }
> } */
> --
> 2.7.4
>
