Patchwork [aarch64] Fix asm-subreg-1.c ICE

login
register
mail settings
Submitter Hurugalawadi, Naveen
Date Nov. 28, 2012, 1:18 p.m.
Message ID <F3068DEED1A463459E0887A091B15493345379@SN2PRD0710MB372.namprd07.prod.outlook.com>
Download mbox | patch
Permalink /patch/202452/
State New
Headers show

Comments

Hurugalawadi, Naveen - Nov. 28, 2012, 1:18 p.m.
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
Marcus Shawcroft - Nov. 28, 2012, 4:41 p.m.
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

Patch

--- 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;
       }