diff options
author | Jakub Jelinek <jakub@redhat.com> | 2024-03-26 11:21:38 +0100 |
---|---|---|
committer | Jakub Jelinek <jakub@redhat.com> | 2024-03-26 11:21:38 +0100 |
commit | c4f2c84e8fa369856aee76679590eb613724bfb0 (patch) | |
tree | 204173b5933e8f950953edc8a9ca2c0478beb72d /gcc/fold-const.cc | |
parent | 471967ab8b4c49338ba77defbe24b06cc51c0093 (diff) | |
download | gcc-c4f2c84e8fa369856aee76679590eb613724bfb0.zip gcc-c4f2c84e8fa369856aee76679590eb613724bfb0.tar.gz gcc-c4f2c84e8fa369856aee76679590eb613724bfb0.tar.bz2 |
fold-const: Punt on MULT_EXPR in extract_muldiv MIN/MAX_EXPR case [PR111151]
As I've tried to explain in the comments, the extract_muldiv_1
MIN/MAX_EXPR optimization is wrong for code == MULT_EXPR.
If the multiplication is done in unsigned type or in signed
type with -fwrapv, it is fairly obvious that max (a, b) * c
in many cases isn't equivalent to max (a * c, b * c) (or min if c is
negative) due to overflows, but even for signed with undefined overflow,
the optimization could turn something without UB in it (where
say a * c invokes UB, but max (or min) picks the other operand where
b * c doesn't).
As for division/modulo, I think it is in most cases safe, except if
the problematic INT_MIN / -1 case could be triggered, but we can
just punt for MAX_EXPR because for MIN_EXPR if one operand is INT_MIN,
we'd pick that operand already. It is just for completeness, match.pd
already has an optimization which turns x / -1 into -x, so the division
by zero is mostly theoretical. That is also why in the testcase the
i case isn't actually miscompiled without the patch, while the c and f
cases are.
2024-03-26 Jakub Jelinek <jakub@redhat.com>
PR middle-end/111151
* fold-const.cc (extract_muldiv_1) <case MAX_EXPR>: Punt for
MULT_EXPR altogether, or for MAX_EXPR if c is -1.
* gcc.c-torture/execute/pr111151.c: New test.
Diffstat (limited to 'gcc/fold-const.cc')
-rw-r--r-- | gcc/fold-const.cc | 21 |
1 files changed, 21 insertions, 0 deletions
diff --git a/gcc/fold-const.cc b/gcc/fold-const.cc index 299c22b..8960e52 100644 --- a/gcc/fold-const.cc +++ b/gcc/fold-const.cc @@ -7104,6 +7104,27 @@ extract_muldiv_1 (tree t, tree c, enum tree_code code, tree wide_type, if (TYPE_UNSIGNED (ctype) != TYPE_UNSIGNED (type)) break; + /* Punt for multiplication altogether. + MAX (1U + INT_MAX, 1U) * 2U is not equivalent to + MAX ((1U + INT_MAX) * 2U, 1U * 2U), the former is + 0U, the latter is 2U. + MAX (INT_MIN / 2, 0) * -2 is not equivalent to + MIN (INT_MIN / 2 * -2, 0 * -2), the former is + well defined 0, the latter invokes UB. + MAX (INT_MIN / 2, 5) * 5 is not equivalent to + MAX (INT_MIN / 2 * 5, 5 * 5), the former is + well defined 25, the latter invokes UB. */ + if (code == MULT_EXPR) + break; + /* For division/modulo, punt on c being -1 for MAX, as + MAX (INT_MIN, 0) / -1 is not equivalent to + MIN (INT_MIN / -1, 0 / -1), the former is well defined + 0, the latter invokes UB (or for -fwrapv is INT_MIN). + MIN (INT_MIN, 0) / -1 already invokes UB, so the + transformation won't make it worse. */ + else if (tcode == MAX_EXPR && integer_minus_onep (c)) + break; + /* MIN (a, b) / 5 -> MIN (a / 5, b / 5) */ sub_strict_overflow_p = false; if ((t1 = extract_muldiv (op0, c, code, wide_type, |