https://gcc.gnu.org/bugzilla/show_bug.cgi?id=123978

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
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?

Reply via email to