| Message ID | F3068DEED1A463459E0887A091B15493345379@SN2PRD0710MB372.namprd07.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
On 28/11/12 13:18, Hurugalawadi, Naveen wrote: > Hi, > >>> Not to mention the ChangeLog entry. > > Sorry about the missed patch and ChangeLog Entry. > Please review the patch and let me know if its OK. > > Regression Tested on aarch64-elf. No new Regressions. > > 2012-11-28 Naveen H.S <Naveen.Hurugalawadi@caviumnetworks.com> > > * aarch64.c (aarch64_load_symref_appropriately): Handle > SYMBOL_SMALL_ABSOLUTE transfers effectively. > > Regards, > Naveen > Hi, Thanks for looking into this issue. The ICE arises because the 'X' constraint on the inline ASM statement in the test cases permits the combiner to rewrite a whole bunch of stuff ending with this: (insn 9 8 0 2 (asm_operands/v ("") ("") 0 [ (subreg/s/u:HI (reg:SI 73 [ D.2259 ]) 0) ] [ (asm_input:HI ("X") (null):0) ] [] asm-subreg-1.c:13) asm-subreg-1.c:13 -1 (expr_list:REG_DEAD (reg:SI 73 [ D.2259 ]) (nil))) into this: (insn 9 8 0 2 (asm_operands/v ("") ("") 0 [ (mem/v/j/c:HI (symbol_ref:DI ("*.LANCHOR0") [flags 0x182]) [0 _const_32+0 S2 A64]) ] [ (asm_input:HI ("X") (null):0) ] [] asm-subreg-1.c:13) asm-subreg-1.c:13 -1 (nil)) which doesn't seem unreasonable to me given that 'X' basically says allow anything. Subsequently reload bombs because: reload.c:find_reloads_address_part() is not prepared for force_const_mem() (which calls aarch64_cannot_force_const_mem) to return NULL. The patch presented changes the expansion of a symbol load from this: t <- high(s) d <- losum(t, s) to this: d <- high(s) t <- losum(d, s) d <- t ... which prevents the combiner kicking in and the problem goes latent... I think the real problem here is either reload not being prepared to get NULL back from force_const_mem() or aarch64_cannot_force_const_mem() being to restrictive. Cheers /Marcus
--- gcc/config/aarch64/aarch64.c 2012-11-22 16:23:21.147121486 +0530 +++ gcc/config/aarch64/aarch64.c 2012-11-27 16:54:04.412608356 +0530 @@ -515,9 +515,9 @@ aarch64_load_symref_appropriately (rtx d { tmp_reg = gen_reg_rtx (Pmode); } - - emit_move_insn (tmp_reg, gen_rtx_HIGH (Pmode, imm)); - emit_insn (gen_add_losym (dest, tmp_reg, imm)); + emit_move_insn (dest, gen_rtx_HIGH (Pmode, imm)); + emit_insn (gen_add_losym (tmp_reg, dest, imm)); + emit_move_insn (dest, tmp_reg); return; }
Hi, >> Not to mention the ChangeLog entry. Sorry about the missed patch and ChangeLog Entry. Please review the patch and let me know if its OK. Regression Tested on aarch64-elf. No new Regressions. 2012-11-28 Naveen H.S <Naveen.Hurugalawadi@caviumnetworks.com> * aarch64.c (aarch64_load_symref_appropriately): Handle SYMBOL_SMALL_ABSOLUTE transfers effectively. Regards, Naveen