On Tue, Jul 26, 2016 at 02:14:49PM +0200, Georg-Johann Lay wrote:
> >>which returns const0_rtx because reg 18 is set in insn 43 to const0_rtx.
> >>Total outcome is that the right shift of reg:DI 18 is transformed to a
> >>no-op move and cancelled out in the remainder.
> >
> >Why does num_sign_bit_copies return something bigger than 8?
> 
> Because it thinks the last value was const0_rtx (which is incorrect) and 

Why is that incorrect?  See insn 43.  Is there another insn I missed?

> The problem might be that get_last_value() is called for reg:DI 18 and 
> combine's internal bookkeeping is incorrect
> 
> struct reg_stat_type {
>   /* Record last point of death of (hard or pseudo) register n.  */
>   rtx_insn                    *last_death;
> 
>   /* Record last point of modification of (hard or pseudo) register n.  */
>   rtx_insn                    *last_set;
> 
> .last_set is the set for reg:QI 18 but later insns also change hard reg:DI 
> 18, for example the set of reg:QI 19.  This means that the information in 
> reg_stat_type does not tell the whole story for hard regs.
> 
> One fix could be to get the mode precision of the SET from the last_set 
> insn and only use the information if that mode is at least as wide as the 
> mode of the register for which get_last_value is called.  reg_stat_type has 
> .last_set_mode for this purpose (QImode in the test case).

Yes, last_set_mode, and there is last_set_sign_bit_copies too.  Are those
correct?

> >We do have a value, and it is for this bb.
> 
> But not for the complete hard register; it's only for reg:QI 18 which is 
> one byte of reg:DI 18,

The rest of the hard register bits are set to unspecified values.

> thus the code should never reach this point in the 
> first place and return earlier.  For example:
> 
> Index: combine.c
> ===================================================================
> --- combine.c   (revision 238701)
> +++ combine.c   (working copy)
> @@ -13206,6 +13206,13 @@ get_last_value (const_rtx x)
>        && DF_INSN_LUID (rsp->last_set) >= subst_low_luid)
>      return 0;
> 
> +  /* If the lookup is for a hard register make sure that value contains at 
> least
> +     as many bits as x does.  */
> +
> +  if (regno < FIRST_PSEUDO_REGISTER
> +      && GET_MODE_PRECISION (rsp->last_set_mode) < GET_MODE_PRECISION 
> (GET_MODE (x)))
> +    return 0;
> +
>    /* If the value has all its registers valid, return it.  */
>    if (get_last_value_validate (&value, rsp->last_set, rsp->last_set_label, 
>    0))
>      return value;

That might be a bit harsh.

First things first: is the information recorded correct?


Segher

Reply via email to