Wilco Dijkstra via Gcc-patches <gcc-patches@gcc.gnu.org> writes:
> Improve GOT addressing by emitting the instructions as a pair. This reduces
> register pressure and improves code quality. With -fPIC codesize improves by
> 0.65% and SPECINT2017 improves by 0.25%.
>
> Passes bootstrap and regress. OK for commit?
Normally we should only put two instructions in the same define_insn
if there's a specific ABI or architectural reason for not separating
them. Doing it purely for optimisation reasons is going against the
general direction of travel. So I think the first question is: why
don't we simply delay the split until after reload instead, since
that's the more normal way of handling this kind of thing?
Also, the patch means that we use RTL of the form:
(set (reg:PTR R)
(unspec:PTR [(mem:PTR (symbol_ref:PTR S))]
UNSPEC_GOTSMALLPIC))
to represent the move of S into R. This should just be represented as:
(set (reg:PTR R) (symbol_ref:PTR S))
and go through the normal move patterns.
Thanks,
Richard
> ChangeLog:
> 2021-05-05 Wilco Dijkstra <wdijk...@arm.com>
>
> * config/aarch64/aarch64.md (ldr_got_small_<mode>): Emit ADRP+LDR GOT
> sequence.
> (ldr_got_small_sidi): Likewise.
> * config/aarch64/aarch64.c (aarch64_load_symref_appropriately):
> Remove tmp_reg.
> (aarch64_print_operand): Correctly print got_lo12 in L specifier.
>
> ---
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index
> 641c83b479e76cbcc75b299eb7ae5f634d9db7cd..32c5c76d3c001a79d2a69b7f8243f1f1f605f901
> 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -3625,27 +3625,21 @@ aarch64_load_symref_appropriately (rtx dest, rtx imm,
>
> rtx insn;
> rtx mem;
> - rtx tmp_reg = dest;
> machine_mode mode = GET_MODE (dest);
>
> - if (can_create_pseudo_p ())
> - tmp_reg = gen_reg_rtx (mode);
> -
> - emit_move_insn (tmp_reg, gen_rtx_HIGH (mode, imm));
> if (mode == ptr_mode)
> {
> if (mode == DImode)
> - insn = gen_ldr_got_small_di (dest, tmp_reg, imm);
> + insn = gen_ldr_got_small_di (dest, imm);
> else
> - insn = gen_ldr_got_small_si (dest, tmp_reg, imm);
> + insn = gen_ldr_got_small_si (dest, imm);
>
> mem = XVECEXP (SET_SRC (insn), 0, 0);
> }
> else
> {
> gcc_assert (mode == Pmode);
> -
> - insn = gen_ldr_got_small_sidi (dest, tmp_reg, imm);
> + insn = gen_ldr_got_small_sidi (dest, imm);
> mem = XVECEXP (XEXP (SET_SRC (insn), 0), 0, 0);
> }
>
> @@ -11019,7 +11013,7 @@ aarch64_print_operand (FILE *f, rtx x, int code)
> switch (aarch64_classify_symbolic_expression (x))
> {
> case SYMBOL_SMALL_GOT_4G:
> - asm_fprintf (asm_out_file, ":lo12:");
> + asm_fprintf (asm_out_file, ":got_lo12:");
> break;
>
> case SYMBOL_SMALL_TLSGD:
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index
> abfd84526745d029ad4953eabad6dd17b159a218..36c5c054f86e9cdd1f0945cdbc1beb47aa7ad80a
> 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -6705,25 +6705,23 @@ (define_insn "add_losym_<mode>"
>
> (define_insn "ldr_got_small_<mode>"
> [(set (match_operand:PTR 0 "register_operand" "=r")
> - (unspec:PTR [(mem:PTR (lo_sum:PTR
> - (match_operand:PTR 1 "register_operand" "r")
> - (match_operand:PTR 2 "aarch64_valid_symref"
> "S")))]
> + (unspec:PTR [(mem:PTR (match_operand:PTR 1 "aarch64_valid_symref" "S"))]
> UNSPEC_GOTSMALLPIC))]
> ""
> - "ldr\\t%<w>0, [%1, #:got_lo12:%c2]"
> - [(set_attr "type" "load_<ldst_sz>")]
> + "adrp\\t%0, %A1\;ldr\\t%<w>0, [%0, %L1]"
> + [(set_attr "type" "load_<ldst_sz>")
> + (set_attr "length" "8")]
> )
>
> (define_insn "ldr_got_small_sidi"
> [(set (match_operand:DI 0 "register_operand" "=r")
> (zero_extend:DI
> - (unspec:SI [(mem:SI (lo_sum:DI
> - (match_operand:DI 1 "register_operand" "r")
> - (match_operand:DI 2 "aarch64_valid_symref" "S")))]
> + (unspec:SI [(mem:SI (match_operand:DI 1 "aarch64_valid_symref" "S"))]
> UNSPEC_GOTSMALLPIC)))]
> "TARGET_ILP32"
> - "ldr\\t%w0, [%1, #:got_lo12:%c2]"
> - [(set_attr "type" "load_4")]
> + "adrp\\t%0, %A1\;ldr\\t%w0, [%0, %L1]"
> + [(set_attr "type" "load_4")
> + (set_attr "length" "8")]
> )
>
> (define_insn "ldr_got_small_28k_<mode>"
