Need help with PR71976 combine.c::get_last_value returns a wrong result

[Georg-Johann Lay]

Segher Boessenkool schrieb:
> On Tue, Jul 26, 2016 at 03:38:18PM +0200, Georg-Johann Lay wrote:
>>>> @@ -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.
>> In what respect?
> 
> It disables all kinds of optimisations that work now.
> 
>>> First things first: is the information recorded correct?
>> I think yes, but care must be taken what may be concluded from that 
>> information.  From a set of 8 bits you cannot draw conclusion about all 64 
>> bits; this should be obvious enough :-)
> 
> :-)
> 
>> In the above case rsp[regno] holds only information for 1 sub-byte.  In 
>> order to get the complete DImode story we would have to get the info for 
>> all sub-parts and then put them together...
> 
> Yes, and the rsp stuff does not do that.
> 
> I am testing the following patch.  Thanks,
> 
> 
> Segher
> 
> 
> diff --git a/gcc/combine.c b/gcc/combine.c
> index 77e0d2b..dec6226 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -9977,6 +9977,9 @@ reg_num_sign_bit_copies_for_combine (const_rtx x, machine_mode mode,
>        return NULL;
>      }
>  
> +  if (GET_MODE_PRECISION (rsp->last_set_mode) != GET_MODE_PRECISION (mode))
> +    return NULL;
> +
>    tem = get_last_value (x);
>    if (tem != 0)
>      return tem;

But the problem is inside get_last_value.  You'd have to add such a 
check at /all/ call sites of that function.  Using a value for a hard 
reg that's been set in a smaller mode than the mode of get_last_value is 
just wrong.

For example combine tracks bits which are known to be zero, and if 
get_last_value is used in a similar situation, then the conclusion about 
zero-bits is also wrong.

Would be interesting to patch get_last_value so that it issues some 
diagnostic if

if (regno < FIRST_PSEUDO_REGISTER
     && (GET_MODE_PRECISION (rsp->last_set_mode)
         < GET_MODE_PRECISION (GET_MODE (x))))

and then run the testsuite against that compiler.  I would expect that 
the condition doesn't even trigger on x86.

Johann