[PATCH] Fix ICE when expanding incorrect shift counts (PR rtl-optimization/69764)

[Bernd Schmidt]

On 02/12/2016 12:26 AM, Jakub Jelinek wrote:
> When expanding shifts with invalid shift counts (negative or too large),
> the shift count on the GIMPLE level is typically an int mode INTEGER_CST,
> but when it is passed down through various layers up to
> expand_binop_directly, we only have one known mode (other than operand
> modes, but that is VOIDmode for CONST_INTs) - the mode of the first argument
> (== result).  But, we treat it as if even the shift count has that mode,
> and either keep it as is (if the expander has that mode for the shift
> count), or convert_modes it to the mode of the operand.
> If the CONST_INT is too large, we can have a problem though, because it
> could be e.g result of expand_normal of SImode value originally, but
> we then treat it as valid HImode or QImode CONST_INT, and so either crash
> in convert_modes, or later on when dealing with the shift count, as it
> might not be valid for the mode we are expecting.
> As expand_shift_1 and expand_binop seem to use GEN_INT for the shift count
> in lots of places, rather than say gen_int_mode, I think this needs to be
> fixed up only at the low level - in expand_binop_directly, which this patch
> does.  The common case, where the shift count is >= 0 and < bitsize,
> are handled without need to call gen_int_mode.

Hmm, I'm finding this explanation somewhat confusing.

Isn't the problem just that shifts are unusual as binops, in that the 
two operands can have different modes? We have the appropriate mode for 
the shift count in xmode1, so I think we should just always rebuild a 
CONST_INT in that mode. As in,

   else if (CONST_INT_P (xop1))
     /* Shifts can have different modes for the shift count, and the caller
        might not have taken this into account when generating an 
integer.  */
     xop1 = gen_int_mode (INTVAL (xop1), xmode1);

Ok with that change if it passes.


Bernd