Re: Invalid reload inheritance with paradoxical subregs

[Uros Bizjak <ubizjak at gmail dot com>]

On Wed, Apr 1, 2009 at 3:34 PM, Joern Rennecke <joernr@arc.com> wrote:

> I suggest you first find out more what is exactly reloaded and where
> the inheritance occurs - inheritance can be done by choose_reload_regs
> or later in emit_reload_insns and its subfunctions.
>
> I.e. set a breakpoint on find_reloads and make it conditional on
> insn->u.fld[0].rt_int == 121 && replace
> , let the compilation break there, finish, and call debug_reload()
> to look at the scheduled reloads.
> Then set a breakpoint on choose_reload_regs, make it conditional on
> chain->insn->u.fld[0].rt_int == 121
> , continue to the breakpoint, finish, and call debug_reload() again.
> This should tell you what MEM is actually reloaded, and if the inheritance
> already happens there; depending on what you find, you have to look further
> in choose_reload_regs or in the reload emitting code.

Joern,

thans for your detailed instructions. I think I found, where problem is:

Combine simplifies lshiftrt/shift/and combined instruction under the
assumption, that for ZERO_EXTEND LOAD_EXTEND_OP targets it can prove
that certain bits are zero, so AND operation can be omitted. The
resulting instruction is valid only for memory operand:

(gdb) p debug_rtx (insn)
(insn 121 120 125 2 t.c:22 (set (reg:SI 6 r6 [orig:48 y$k.115 ] [48])
        (lshiftrt:SI (subreg:SI (mem/s/c:HI (plus:SI (reg/f:SI 1 r1)
                        (const_int 16 [0x10])) [0+4 S2 A32]) 0)
            (const_int 5 [0x5]))) 62 {lshrsi3} (nil))

Now, tracing this instruction through reload_as_needed () function, we
have following reloads just after the call to choose_reload_regs
(...):

(gdb) p debug_reload ()
Reload 0: reload_in (HI) = (mem/s/c:HI (plus:SI (reg/f:SI 1 r1)
                                                        (const_int 16
[0x10])) [0+4 S2 A32])
	GENERAL_REGS, RELOAD_FOR_INPUT (opnum = 1)
	reload_in_reg: (mem/s/c:HI (plus:SI (reg/f:SI 1 r1)
                                                        (const_int 16
[0x10])) [0+4 S2 A32])
	reload_reg_rtx: (reg:HI 7 r7)

This is all correct, until subst_reloads (...) is called a coouple of
lines down the source. This call changes (insn 121) to:

(gdb) p debug_rtx (insn)
(insn 121 120 125 2 t.c:22 (set (reg:SI 6 r6 [orig:48 y$k.115 ] [48])
        (lshiftrt:SI (subreg:SI (reg:HI 7 r7) 0)
            (const_int 5 [0x5]))) 62 {lshrsi3} (nil))

This substitution is wrong. Paradoxical subreg  of a memory operand is
for our target known to zero extend to SImode value, however -
paradoxical subreg of a register operand has undefined bits outside
the reg. So, combine and reload doesn't agree on the bits outside the
register.

After this substitution, subreg RTX is simply stripped away via the
call to cleanup_subreg_operands () in reload () and that leaves us
with undefined bits shifted in.

I guess that somehow we have to prevent choose_reload_regs to blindly
substitute memory_operand with register when paradoxical subregs are
involved. The condition for the substitution should be similar to the
condition in nonzero_bits1 from final.c:

#if defined (WORD_REGISTER_OPERATIONS) && defined (LOAD_EXTEND_OP)
	  /* If this is a typical RISC machine, we only have to worry
	     about the way loads are extended.  */
	  if ((LOAD_EXTEND_OP (GET_MODE (SUBREG_REG (x))) == SIGN_EXTEND
	       ? (((nonzero
		    & (((unsigned HOST_WIDE_INT) 1
			<< (GET_MODE_BITSIZE (GET_MODE (SUBREG_REG (x))) - 1))))
		   != 0))
	       : LOAD_EXTEND_OP (GET_MODE (SUBREG_REG (x))) != ZERO_EXTEND)
	      || !MEM_P (SUBREG_REG (x)))
#endif

Uros.