Re: Record equivalences for spill registers

["Richard Sandiford via gcc-patches" <gcc-patches at gcc dot gnu dot org>]

Andrew Pinski <pinskia@gmail.com> writes:
> On Sun, May 7, 2017 at 9:30 PM, Jim Wilson <jim.wilson@linaro.org> wrote:
>> On 05/05/2017 12:23 AM, Richard Sandiford wrote:
>>>
>>> 2017-05-05  Richard Sandiford  <richard.sandiford@linaro.org>
>>>
>>> gcc/
>>>         * lra-constraints.c (lra_copy_reg_equiv): New function.
>>>         (split_reg): Use it to copy equivalence information from the
>>>         original register to the spill register.
>>
>>
>> This patch breaks aarch64 bootstrap.  I get a link error for lto1 and f951

Really sorry for the breakage.  I'd forgotten that this depended on:

  https://gcc.gnu.org/ml/gcc-patches/2017-03/msg01550.html

although it looks like the exact failure I'd originally seen doesn't
trigger with current sources (at least, it didn't reproduce in my testing).

>> godump.o: In function `go_define(unsigned int, char const*)':
>> godump.c:(.text+0x36c): relocation truncated to fit:
>> R_AARCH64_ADR_PREL_LO21 against `.rodata'
>> godump.c:(.text+0x4b4): relocation truncated to fit: R_AARCH64_ADR_PREL_LO21
>> against `.rodata'
>>
>> The godump.c.271r.ira file looks OK, I see
>>
>> (insn 237 223 225 10 (set (reg/f:DI 489)
>>         (high:DI (label_ref 240))) "../../gcc-svn/gcc/godump.c":174 49
>> {*movdi_aarch64}
>>      (expr_list:REG_EQUIV (high:DI (label_ref 240))
>>         (insn_list:REG_LABEL_OPERAND 240 (nil))))
>> ...
>> (insn 238 115 1157 10 (set (reg/f:DI 490)
>>         (lo_sum:DI (reg/f:DI 489)
>>             (label_ref 240))) "../../gcc-svn/gcc/godump.c":174 929
>> {add_losym_di}
>>      (expr_list:REG_DEAD (reg/f:DI 489)
>>         (expr_list:REG_EQUIV (label_ref 240)
>>             (insn_list:REG_LABEL_OPERAND 240 (nil)))))
>>
>> But in the godump.c.272r.reload file I see in a different basic block
>>
>> (insn 1244 76 1161 22 (set (reg/f:DI 7 x7 [490])
>>         (label_ref 240)) "../../gcc-svn/gcc/godump.c":221 49
>> {*movdi_aarch64}
>>      (nil))
>>
>> which is not OK.  This label ref is the address of a jumptable in the rodata
>> section, and can't be loaded with a single instruction.  It looks like there
>> needs to be some extra work when rematerializing, to handle equiv values
>> that can't just be copied to a register.
>
> Hmm, shouldn't have the mov rejected as being invalid?  I think that
> is one issue with the aarch64 backend there.
>
> See https://gcc.gnu.org/ml/gcc-patches/2014-09/msg01800.html
>
>
> I can't remember if the following patch was ever submitted or committed.
>
> Here are my notes about this patch from the internal bug report we got
> here at Cavium (back in 2013):
>
> Switch tables are implemented using the tiny model but they are stored
> in the rodata section which means they could overflow the address.
>
> Note this patch most likely won't apply directly either:
>
> From: Andrew Pinski <apinski@cavium.com>
> Date: Thu, 15 Aug 2013 20:42:12 +0000 (-0700)
> Subject: 2013-08-11  Andrew Pinski  <apinski@cavium.com>
> X-Git-Tag: tc47_SDK_3_1_0_build_28~9
> X-Git-Url: http://cagit1.caveonetworks.com/git/?p=toolchain%2Fgcc.git;a=commitdiff_plain;h=1714f167b575956801264db5cd2300203a58e3e9
>
> 2013-08-11  Andrew Pinski  <apinski@cavium.com>
>
> Bug #7079
> * config/aarch64/aarch64.md (*movsi_aarch64): Use Ust instead of S constrain.
> (*movdi_aarch64): Likewise.
> (ldr_got_small_sidi): Add type attribute.
> * config/aarch64/constraints.md (Ust): New constraint like S but only
> if the symbol is SYMBOL_TINY_ABSOLUTE.
> ---
>
> diff --git a/gcc/ChangeLog.aarch64 b/gcc/ChangeLog.aarch64
> index 3c8ff13..f4e1e91 100644
> --- a/gcc/ChangeLog.aarch64
> +++ b/gcc/ChangeLog.aarch64
> @@ -1,3 +1,12 @@
> +2013-08-11  Andrew Pinski  <apinski@cavium.com>
> +
> + Bug #7079
> + * config/aarch64/aarch64.md (*movsi_aarch64): Use Ust instead of S constrain.
> + (*movdi_aarch64): Likewise.
> + (ldr_got_small_sidi): Add type attribute.
> + * config/aarch64/constraints.md (Ust): New constraint like S but only
> + if the symbol is SYMBOL_TINY_ABSOLUTE.
> +
>  2013-08-14  Yufeng Zhang  <yufeng.zhang@arm.com>
>
>   * function.c (assign_parm_find_data_types): Set passed_mode and
> diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
> index 485bd59..0cd6a41 100644
> --- a/gcc/config/aarch64/aarch64.md
> +++ b/gcc/config/aarch64/aarch64.md
> @@ -810,7 +810,7 @@
>
>  (define_insn "*movsi_aarch64"
>    [(set (match_operand:SI 0 "nonimmediate_operand" "=r,k,r,r,r,*w,m,
> m,r,r  ,*w, r,*w")
> - (match_operand:SI 1 "aarch64_mov_operand"  " r,r,k,M,m,
> m,rZ,*w,S,Ush,rZ,*w,*w"))]
> + (match_operand:SI 1 "aarch64_mov_operand"  " r,r,k,M,m,
> m,rZ,*w,Ust,Ush,rZ,*w,*w"))]
>    "(register_operand (operands[0], SImode)
>      || aarch64_reg_or_zero (operands[1], SImode))"
>    "@
> @@ -836,7 +836,7 @@
>
>  (define_insn "*movdi_aarch64"
>    [(set (match_operand:DI 0 "nonimmediate_operand" "=r,k,r,r,r,*w,m,
> m,r,r,  *w, r,*w,w")
> - (match_operand:DI 1 "aarch64_mov_operand"  " r,r,k,N,m,
> m,rZ,*w,S,Ush,rZ,*w,*w,Dd"))]
> + (match_operand:DI 1 "aarch64_mov_operand"  " r,r,k,N,m,
> m,rZ,*w,Ust,Ush,rZ,*w,*w,Dd"))]
>    "(register_operand (operands[0], DImode)
>      || aarch64_reg_or_zero (operands[1], DImode))"
>    "@
> @@ -3978,6 +3978,7 @@
>    "TARGET_ILP32"
>    "ldr\\t%w0, [%1, #:got_lo12:%a2]"
>    [(set_attr "v8type" "load1")
> +   (set_attr "type" "load1")
>     (set_attr "mode" "DI")]
>  )
>
> diff --git a/gcc/config/aarch64/constraints.md
> b/gcc/config/aarch64/constraints.md
> index 2570400..a36bdaf 100644
> --- a/gcc/config/aarch64/constraints.md
> +++ b/gcc/config/aarch64/constraints.md
> @@ -114,6 +114,12 @@
>    (and (match_code "const_int")
>         (match_test "(unsigned) exact_log2 (ival) <= 4")))
>
> +(define_constraint "Ust"
> +  "A constraint that matches an absolute symbolic address; only for
> tiny model."
> +  (and (match_code "const,symbol_ref,label_ref")
> +       (match_test "aarch64_symbolic_address_p (op)
> +    && aarch64_classify_symbolic_expression (op, SYMBOL_CONTEXT_ADR)
> == SYMBOL_TINY_ABSOLUTE")))
> +
>  (define_memory_constraint "Q"
>   "A memory address which uses a single base register with no offset."
>   (and (match_code "mem")

Looks like this is functionally equivalent to my patch, although it
obviously predates it by quite some way.

Thanks,
Richard