Hi!
This case is correctly described in the comment in expand_mul_overflow:
s1 * s2 -> ur
t1 = (s1 & s2) < 0 ? (-(U) s1) : ((U) s1)
t2 = (s1 & s2) < 0 ? (-(U) s2) : ((U) s2)
res = t1 * t2
ovf = (s1 ^ s2) < 0 ? (s1 && s2) : main_ovf (true) */
where if one of the operands is negative and the other non-negative,
the overflow is whenever both operands are non-zero, so s1 && s2,
but unfortunately the code has been implementing s1 & s2 instead,
which e.g. for the -2 and 1 where (-2 & 1) == 0, but (-2 && 1) != 0
is incorrect.
The second hunk is when we know from VRP that one of the operand (and which
one) is negative and which one is non-negative, obviously the negative
one can't be zero, so we can just compare the non-negative one against zero.
The third hunk is when we don't know this precisely from VRP. In that case
we used to do essentially
if ((s1 & s2) == 0)
goto main_label;
ovf = 1;
main_label:
ovf |= __builtin_mul_overflow ((unsigned) s1, (unsigned) s2, &r);
(the fallthrough is ok, because mul_overflow when one of the operands
is zero will just compute zero), this patch changes it to:
if (s1 == 0)
goto main_label;
if (s2 == 0)
goto main_label;
ovf = 1;
main_label:
ovf |= __builtin_mul_overflow ((unsigned) s1, (unsigned) s2, &r);
but as an optimization, if we know from VRP that one of the operands
is negative, we don't bother comparing that operand to 0.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk
and release branches?
2019-11-19 Jakub Jelinek <ja...@redhat.com>
PR middle-end/91450
* internal-fn.c (expand_mul_overflow): For s1 * s2 -> ur, if one
operand is negative and one non-negative, compare the non-negative
one against 0 rather than comparing s1 & s2 against 0. Otherwise,
don't compare (s1 & s2) == 0, but compare separately both s1 == 0
and s2 == 0, unless one of them is known to be negative. Remove
tem2 variable, use tem where tem2 has been used before.
* gcc.c-torture/execute/pr91450-1.c: New test.
* gcc.c-torture/execute/pr91450-2.c: New test.
--- gcc/internal-fn.c.jj 2019-11-08 11:33:48.000000000 +0100
+++ gcc/internal-fn.c 2019-11-18 14:41:48.247381649 +0100
@@ -1408,7 +1408,7 @@ expand_mul_overflow (location_t loc, tre
/* s1 * s2 -> ur */
if (!uns0_p && !uns1_p && unsr_p)
{
- rtx tem, tem2;
+ rtx tem;
switch (pos_neg0 | pos_neg1)
{
case 1: /* Both operands known to be non-negative. */
@@ -1438,10 +1438,8 @@ expand_mul_overflow (location_t loc, tre
ops.op2 = NULL_TREE;
ops.location = loc;
res = expand_expr_real_2 (&ops, NULL_RTX, mode, EXPAND_NORMAL);
- tem = expand_binop (mode, and_optab, op0, op1, NULL_RTX, false,
- OPTAB_LIB_WIDEN);
- do_compare_rtx_and_jump (tem, const0_rtx, EQ, true, mode,
- NULL_RTX, NULL, done_label,
+ do_compare_rtx_and_jump (pos_neg0 == 1 ? op0 : op1, const0_rtx,
EQ,
+ true, mode, NULL_RTX, NULL, done_label,
profile_probability::very_likely ());
goto do_error_label;
}
@@ -1472,16 +1470,23 @@ expand_mul_overflow (location_t loc, tre
arg1 = error_mark_node;
emit_jump (do_main_label);
emit_label (after_negate_label);
- tem2 = expand_binop (mode, xor_optab, op0, op1, NULL_RTX, false,
- OPTAB_LIB_WIDEN);
- do_compare_rtx_and_jump (tem2, const0_rtx, GE, false, mode, NULL_RTX,
- NULL, do_main_label,
profile_probability::very_likely ());
+ tem = expand_binop (mode, xor_optab, op0, op1, NULL_RTX, false,
+ OPTAB_LIB_WIDEN);
+ do_compare_rtx_and_jump (tem, const0_rtx, GE, false, mode, NULL_RTX,
+ NULL, do_main_label,
+ profile_probability::very_likely ());
/* One argument is negative here, the other positive. This
overflows always, unless one of the arguments is 0. But
if e.g. s2 is 0, (U) s1 * 0 doesn't overflow, whatever s1
is, thus we can keep do_main code oring in overflow as is. */
- do_compare_rtx_and_jump (tem, const0_rtx, EQ, true, mode, NULL_RTX,
- NULL, do_main_label,
profile_probability::very_likely ());
+ if (pos_neg0 != 2)
+ do_compare_rtx_and_jump (op0, const0_rtx, EQ, true, mode, NULL_RTX,
+ NULL, do_main_label,
+ profile_probability::very_unlikely ());
+ if (pos_neg1 != 2)
+ do_compare_rtx_and_jump (op1, const0_rtx, EQ, true, mode, NULL_RTX,
+ NULL, do_main_label,
+ profile_probability::very_unlikely ());
expand_arith_set_overflow (lhs, target);
emit_label (do_main_label);
goto do_main;
--- gcc/testsuite/gcc.c-torture/execute/pr91450-1.c.jj 2019-11-18
14:32:04.156108293 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr91450-1.c 2019-11-18
14:39:28.693466446 +0100
@@ -0,0 +1,88 @@
+/* PR middle-end/91450 */
+
+__attribute__((noipa)) unsigned long long
+foo (int a, int b)
+{
+ unsigned long long r;
+ if (!__builtin_mul_overflow (a, b, &r))
+ __builtin_abort ();
+ return r;
+}
+
+__attribute__((noipa)) unsigned long long
+bar (int a, int b)
+{
+ unsigned long long r;
+ if (a >= 0)
+ return 0;
+ if (!__builtin_mul_overflow (a, b, &r))
+ __builtin_abort ();
+ return r;
+}
+
+__attribute__((noipa)) unsigned long long
+baz (int a, int b)
+{
+ unsigned long long r;
+ if (b >= 0)
+ return 0;
+ if (!__builtin_mul_overflow (a, b, &r))
+ __builtin_abort ();
+ return r;
+}
+
+__attribute__((noipa)) unsigned long long
+qux (int a, int b)
+{
+ unsigned long long r;
+ if (a >= 0)
+ return 0;
+ if (b < 0)
+ return 0;
+ if (!__builtin_mul_overflow (a, b, &r))
+ __builtin_abort ();
+ return r;
+}
+
+__attribute__((noipa)) unsigned long long
+quux (int a, int b)
+{
+ unsigned long long r;
+ if (a < 0)
+ return 0;
+ if (b >= 0)
+ return 0;
+ if (!__builtin_mul_overflow (a, b, &r))
+ __builtin_abort ();
+ return r;
+}
+
+int
+main ()
+{
+ if (foo (-4, 2) != -8ULL)
+ __builtin_abort ();
+ if (foo (2, -4) != -8ULL)
+ __builtin_abort ();
+ if (bar (-4, 2) != -8ULL)
+ __builtin_abort ();
+ if (baz (2, -4) != -8ULL)
+ __builtin_abort ();
+ if (qux (-4, 2) != -8ULL)
+ __builtin_abort ();
+ if (quux (2, -4) != -8ULL)
+ __builtin_abort ();
+ if (foo (-2, 1) != -2ULL)
+ __builtin_abort ();
+ if (foo (1, -2) != -2ULL)
+ __builtin_abort ();
+ if (bar (-2, 1) != -2ULL)
+ __builtin_abort ();
+ if (baz (1, -2) != -2ULL)
+ __builtin_abort ();
+ if (qux (-2, 1) != -2ULL)
+ __builtin_abort ();
+ if (quux (1, -2) != -2ULL)
+ __builtin_abort ();
+ return 0;
+}
--- gcc/testsuite/gcc.c-torture/execute/pr91450-2.c.jj 2019-11-18
14:32:12.204987999 +0100
+++ gcc/testsuite/gcc.c-torture/execute/pr91450-2.c 2019-11-18
14:33:37.881707491 +0100
@@ -0,0 +1,76 @@
+/* PR middle-end/91450 */
+
+__attribute__((noipa)) void
+foo (int a, int b)
+{
+ unsigned long long r;
+ if (__builtin_mul_overflow (a, b, &r))
+ __builtin_abort ();
+ if (r != 0)
+ __builtin_abort ();
+}
+
+__attribute__((noipa)) void
+bar (int a, int b)
+{
+ unsigned long long r;
+ if (a >= 0)
+ return;
+ if (__builtin_mul_overflow (a, b, &r))
+ __builtin_abort ();
+ if (r != 0)
+ __builtin_abort ();
+}
+
+__attribute__((noipa)) void
+baz (int a, int b)
+{
+ unsigned long long r;
+ if (b >= 0)
+ return;
+ if (__builtin_mul_overflow (a, b, &r))
+ __builtin_abort ();
+ if (r != 0)
+ __builtin_abort ();
+}
+
+__attribute__((noipa)) void
+qux (int a, int b)
+{
+ unsigned long long r;
+ if (a >= 0)
+ return;
+ if (b < 0)
+ return;
+ if (__builtin_mul_overflow (a, b, &r))
+ __builtin_abort ();
+ if (r != 0)
+ __builtin_abort ();
+}
+
+__attribute__((noipa)) void
+quux (int a, int b)
+{
+ unsigned long long r;
+ if (a < 0)
+ return;
+ if (b >= 0)
+ return;
+ if (__builtin_mul_overflow (a, b, &r))
+ __builtin_abort ();
+ if (r != 0)
+ __builtin_abort ();
+}
+
+int
+main ()
+{
+ foo (-4, 0);
+ foo (0, -4);
+ foo (0, 0);
+ bar (-4, 0);
+ baz (0, -4);
+ qux (-4, 0);
+ quux (0, -4);
+ return 0;
+}
Jakub
