Re: [PATCH v2] AArch64: Improve GOT addressing

[Richard Sandiford <richard.sandiford@arm.com>]

Wilco Dijkstra <Wilco.Dijkstra@arm.com> writes:
> @@ -23746,6 +23767,24 @@ aarch_macro_fusion_pair_p (rtx_insn *prev, rtx_insn *curr)
>          }
>      }
>
> +  /* Always treat GOT accesses as a pair to ensure they can be easily
> +     identified and optimized in linkers.  */

Sorry, I guess I'd not given enough weight to this part from your
earlier reply.  I was focusing too much on “Well there are no
optimizations that benefit from them being split, and there is
no gain from scheduling them independently.”

Are we actually planning to do any linker relaxations here, or is this
purely theoretical?  If doing relaxations is a realistic possiblity then
I agree that would be a good/legitimate reason to use a single define_insn
for both instructions.  In that case though, there should be a comment
above the define_insn explaining that linker relaxation is the reason
for keeping the instructions together.

If the relaxations aren't a realistic possibilty then I don't think we
want this fusion change either.

Thanks,
Richard

> +  if (simple_sets_p)
> +    {
> +      /*  We're trying to match:
> +         prev (adrp) == (set (reg r1) (high (symbol_ref ("SYM"))))
> +         curr (add) == (set (reg r0)
> +                       (unspec [(mem (lo_sum (reg r1) (symbol_ref ("SYM"))))]
> +                        UNSPEC_GOTSMALLPIC))  */
> +
> +      if (satisfies_constraint_Ush (SET_SRC (prev_set))
> +         && REG_P (SET_DEST (prev_set))
> +         && REG_P (SET_DEST (curr_set))
> +         && GET_CODE (SET_SRC (curr_set)) == UNSPEC
> +         && XINT (SET_SRC (curr_set), 1) == UNSPEC_GOTSMALLPIC)
> +       return true;
> +    }
> +
>    if (simple_sets_p && aarch64_fusion_enabled_p (AARCH64_FUSE_MOVK_MOVK))
>      {
>
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index abfd84526745d029ad4953eabad6dd17b159a218..2527c96576a78f2071da20721143a27adeb1551b 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -1283,8 +1283,11 @@ (define_insn_and_split "*movsi_aarch64"
>     fmov\\t%w0, %s1
>     fmov\\t%s0, %s1
>     * return aarch64_output_scalar_simd_mov_immediate (operands[1], SImode);"
> -  "CONST_INT_P (operands[1]) && !aarch64_move_imm (INTVAL (operands[1]), SImode)
> -    && REG_P (operands[0]) && GP_REGNUM_P (REGNO (operands[0]))"
> +  "(CONST_INT_P (operands[1]) && !aarch64_move_imm (INTVAL (operands[1]), SImode)
> +    && REG_P (operands[0]) && GP_REGNUM_P (REGNO (operands[0])))
> +    || (reload_completed
> +       && (aarch64_classify_symbolic_expression (operands[1])
> +           == SYMBOL_SMALL_GOT_4G))"
>     [(const_int 0)]
>     "{
>         aarch64_expand_mov_immediate (operands[0], operands[1]);
> @@ -1319,8 +1322,11 @@ (define_insn_and_split "*movdi_aarch64"
>     fmov\\t%x0, %d1
>     fmov\\t%d0, %d1
>     * return aarch64_output_scalar_simd_mov_immediate (operands[1], DImode);"
> -   "(CONST_INT_P (operands[1]) && !aarch64_move_imm (INTVAL (operands[1]), DImode))
> -    && REG_P (operands[0]) && GP_REGNUM_P (REGNO (operands[0]))"
> +   "(CONST_INT_P (operands[1]) && !aarch64_move_imm (INTVAL (operands[1]), DImode)
> +    && REG_P (operands[0]) && GP_REGNUM_P (REGNO (operands[0])))
> +    || (reload_completed
> +       && (aarch64_classify_symbolic_expression (operands[1])
> +           == SYMBOL_SMALL_GOT_4G))"
>     [(const_int 0)]
>     "{
>         aarch64_expand_mov_immediate (operands[0], operands[1]);