On Sun, May 7, 2017 at 10:26 PM, Andrew Pinski <pins...@gmail.com> wrote:
> On Sun, May 7, 2017 at 9:30 PM, Jim Wilson <jim.wil...@linaro.org> wrote:
>> On 05/05/2017 12:23 AM, Richard Sandiford wrote:
>>>
>>> 2017-05-05 Richard Sandiford <richard.sandif...@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
>>
>> 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.
Some more notes:
(In reply to comment #15)
> (In reply to comment #14)
> > //(insn 793 35 795 (set (reg/f:DI 2 x2 [450])
> > // (label_ref 456)) 32 {*movdi_aarch64}
> > // (insn_list:REG_LABEL_OPERAND 456 (expr_list:REG_EQUAL (label_ref 456)
> > // (nil))))
> > adr x2, .L806 // 793 *movdi_aarch64/9 [length = 4]
>
> Which is generated by the register allocator and we never split it into the
> adrp/add again.
The register allocator is doing an ok job as the backend said this is
a valid pattern. We need a constraint for
*movdi_aarch64/*movsi_aarch64 which says this is not a valid pattern
and to go through the standard gen_movinsn path.
Thanks,
Andrew Pinski
>
> Note this patch most likely won't apply directly either:
>
> From: Andrew Pinski <apin...@cavium.com>
> Date: Thu, 15 Aug 2013 20:42:12 +0000 (-0700)
> Subject: 2013-08-11 Andrew Pinski <apin...@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 <apin...@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 <apin...@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.zh...@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")
>
>
>>
>> I haven't had a chance to step through this in a debugger to see what is
>> going on yet.
>>
>> Jim
>>
