On Tue, Mar 26, 2013 at 6:30 PM, Frederic Riss <frederic.r...@gmail.com> wrote:
> I was playing with adding support of the various modes of widening
> multiplies on my backend, and hit some restrictions in the expansion
> code that I couldn't explain to myself. These restrictions only impact
> the signed by unsigned version.
>
> The first limitation was about the detection of widening multiplies
> when one of the operands is a big constant of opposite signedness of
> the other. It might very well be the case that nobody cared adding the
> support for that. I used the following simple patch to overcome that:
>
> @@ -2059,16 +2059,30 @@ is_widening_mult_p (gimple stmt,
>
> if (*type1_out == NULL)
> {
> - if (*type2_out == NULL || !int_fits_type_p (*rhs1_out, *type2_out))
> - return false;
> - *type1_out = *type2_out;
> + if (*type2_out == NULL)
> + return false;
> + if (!int_fits_type_p (*rhs1_out, *type2_out)) {
> + tree other_type = signed_or_unsigned_type_for (!TYPE_UNSIGNED
> (*type2_out)
> + *type2_out);
> + if (!int_fits_type_p (*rhs1_out, other_type))
> + return false;
> + *type1_out = other_type;
> + } else {
> + *type1_out = *type2_out;
> + }
> }
>
> if (*type2_out == NULL)
> {
> - if (!int_fits_type_p (*rhs2_out, *type1_out))
> - return false;
> - *type2_out = *type1_out;
> + if (!int_fits_type_p (*rhs2_out, *type1_out)) {
> + tree other_type = signed_or_unsigned_type_for (!TYPE_UNSIGNED
> (*type1_out)
> + *type1_out);
> + if (!int_fits_type_p (*rhs2_out, other_type))
> + return false;
> + *type2_out = other_type;
> + } else {
> + *type2_out = *type1_out;
> + }
> }
>
> Is that extension of the logic correct?
I didn't look at the details, but certainly this place is correct for
extending the range of supported widening operations.
> After having done that modification and thus having the middle end
> generate widening multiplies of this kind, I hit the second limitation
> in expr.c:expand_expr_real_2 :
>
> /* First, check if we have a multiplication of one signed and one
> unsigned operand. */
> if (TREE_CODE (treeop1) != INTEGER_CST
> && (TYPE_UNSIGNED (TREE_TYPE (treeop0))
> != TYPE_UNSIGNED (TREE_TYPE (treeop1))))
>
> Here, the code trying to expand a signed by unsigned widening multiply
> explicitly checks that the operand isn't a constant. Why is that? I
> removed that condition to try to find the failing cases, but the few
> million random multiplies that I threw at it didn't fail in any
> visible way.
Not sure, the limitation does not make sense to me. Probably the
code assumes that it would have been easy to convert the constant
to the same signedness as treeop0. Simply removing the check
seems correct to me.
> One difficulty I found was that the widening multiplies are expressed as eg:
>
> (mult (zero_extend (operand 1)) (zero_extend (operand 2)))
>
> and that simplify_rtx will ICE when trying to simplify a zero_extend
> of a VOIDmode const_int. It forced me to carefully add different
> patterns to handle the immediate versions of the operations. But that
> doesn't seem like a good reason to limit the code expansion...
>
> Can anyone explain this condition?
The zero_extend needs to be simplified for constant operand 2 - the
expansion code probably doesn't do this appropriately.
Richard.
> Many thanks,
> Fred
