https://gcc.gnu.org/bugzilla/show_bug.cgi?id=123978
--- Comment #6 from Tamar Christina <tnfchris at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #3)
> The r13-6617 WIDEN_MULT handling just looks weird.
> The WIDEN_MULT_EXPR documentation says:
> /* Widening multiplication.
> The two arguments are of type t1 and t2, both integral types that
> have the same precision, but possibly different signedness.
> The result is of integral type t3, such that t3 is at least twice
> the size of t1/t2. WIDEN_MULT_EXPR is equivalent to first widening
> (promoting) the arguments from type t1 to type t3, and from t2 to
> type t3 and then multiplying them. */
> DEFTREECODE (WIDEN_MULT_EXPR, "widen_mult_expr", tcc_binary, 2)
> So, we have 8 different cases of t1, t2 and t3 signs.
> Now, maybe_non_standard punts on 2 of those cases (s u -> s and u s -> s)
> and canonicalizes the order of arguments, such that u s -> u becomes s u ->
> u.
> case WIDEN_MULT_EXPR:
> {
> m_op1 = gimple_assign_rhs1 (m_stmt);
> m_op2 = gimple_assign_rhs2 (m_stmt);
> tree ret = gimple_assign_lhs (m_stmt);
> bool signed1 = TYPE_SIGN (TREE_TYPE (m_op1)) == SIGNED;
> bool signed2 = TYPE_SIGN (TREE_TYPE (m_op2)) == SIGNED;
> bool signed_ret = TYPE_SIGN (TREE_TYPE (ret)) == SIGNED;
>
> /* Normally these operands should all have the same sign, but
> some passes and violate this by taking mismatched sign args. At
> the moment the only one that's possible is mismatch inputs and
> unsigned output. Once ranger supports signs for the operands we
> can properly fix it, for now only accept the case we can do
> correctly. */
> if ((signed1 ^ signed2) && signed_ret)
> return;
>
> if (signed2 && !signed1)
> std::swap (m_op1, m_op2);
>
> if (signed1 || signed2)
> m_operator = signed_op.range_op ();
> else
> m_operator = unsigned_op.range_op ();
> break;
> }
> That still is IMHO 5 different cases, s s -> s, s s -> u, u u -> s, u u -> u
> and s u -> u. But the code uses
> OP_WIDEN_MULT_SIGNED for the 3 s s -> s, s s -> u and s u -> u cases and
> OP_WINDE_MULT_UNSIGNED for the 2 u u -> s and u u -> u cases.
> Now, operator_widen_mult_signed::wi_fold uses
> signop s = TYPE_SIGN (type);
>
> wide_int lh_wlb = wide_int::from (lh_lb, wi::get_precision (lh_lb) * 2,
> SIGNED);
> wide_int lh_wub = wide_int::from (lh_ub, wi::get_precision (lh_ub) * 2,
> SIGNED);
> wide_int rh_wlb = wide_int::from (rh_lb, wi::get_precision (rh_lb) * 2, s);
> wide_int rh_wub = wide_int::from (rh_ub, wi::get_precision (rh_ub) * 2, s);
> which looks good for the s s -> s and s u -> u cases but looks wrong for s s
> -> u.
> And then operator_widen_mult_unsigned::wi_fold uses
> signop s = TYPE_SIGN (type);
>
> wide_int lh_wlb = wide_int::from (lh_lb, wi::get_precision (lh_lb) * 2,
> UNSIGNED);
> wide_int lh_wub = wide_int::from (lh_ub, wi::get_precision (lh_ub) * 2,
> UNSIGNED);
> wide_int rh_wlb = wide_int::from (rh_lb, wi::get_precision (rh_lb) * 2, s);
> wide_int rh_wub = wide_int::from (rh_ub, wi::get_precision (rh_ub) * 2, s);
> and that looks correct for u u -> u but looks wrong for u u -> s (the case
> in the testcase).
> So, in the latter case, I'd say the fix should be just to use UNSIGNED
> instead of s for rh_wlb/rh_wub too.
> And for the other case we likely need another case to handle the s s -> u
> case?
Yeah It was brought up back when, but we assumed s s -> u and u u -> s weren't
possible as the only code that could introduce this at the time
convert_mult_to_widen
had
if (from_unsigned1 && from_unsigned2)
op = umul_widen_optab;
else if (!from_unsigned1 && !from_unsigned2)
op = smul_widen_optab;
else
op = usmul_widen_optab;
and the underlying optabs are documented to have uniform types, except for
usmul
which was supposed to only allow a signed result. so s u -> s and u s -> s.
