On Sun, May 7, 2017 at 11:37 PM, Richard Sandiford <richard.sandif...@linaro.org> wrote: > Andrew Pinski <pins...@gmail.com> writes: >> 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 > > 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 <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") > > Looks like this is functionally equivalent to my patch, although it > obviously predates it by quite some way.
I did not even notice you had a similar patch outstanding until you pointed it out. But I think it is obvious as the current code is obviously wrong and we both came up with the same solution (3/4 years apart). Thanks, Andrew > > Thanks, > Richard
