From a2370cc4f47fa7f1b2c72135ee1685551c2e25f4 Mon Sep 17 00:00:00 2001 From: Jakub Jelinek Date: Wed, 27 Nov 2024 14:32:07 +0100 Subject: match.pd: Avoid introducing UB in the ((X /[ex] C1) +- C2) * (C1 * C3) simplification [PR117692] As the pr117692.c testcase shows, the generalized pattern can introduce UB when there wasn't any. The old pattern was I believe correct, it is as if in the new pattern C3 was always 1 and I don't see how that could have introduced UB. But if type is signed and C3 (aka factor) isn't 1 and for + X and C2 could have different sign or for - X and C2 could have the same sign, when doing the addition/subtraction first the absolute value could decrease, while if first multiplying by C3 we could invoke UB already during that multiplication. The following patch fixes it by going through the casts to utype if ranger (get_range_pos_neg) detects the sign compared to sign of C2 (INTEGER_CST) could be the same or could be different depending on op because then the absolute value will not increase. Other possibility (perhaps as another check if this check doesn't succeed) would be to test whether X * C3 could actually overflow. vr-values.cc has check_for_binary_op_overflow (currently not exported) which I think does what we'd need to check, if it returns true and sets ovf to false. 2024-11-27 Jakub Jelinek PR tree-optimization/117692 * tree.cc (get_range_pos_neg): Adjust function comment, use non-negative instead of positive. * match.pd (((X /[ex] C1) +- C2) * (C1 * C3) -> (X * C3) +- (C1 * C2 * C3)): Use casts to utype if type is signed, factor isn't 1 and C1 and C2 could have different sign for + or could have the same sign for -. * gcc.dg/tree-ssa/mulexactdiv-5.c: Expect 8 nop_exprs. * gcc.dg/tree-ssa/pr117692.c: New test. --- gcc/match.pd | 10 +++++++++- gcc/testsuite/gcc.dg/tree-ssa/mulexactdiv-5.c | 2 +- gcc/testsuite/gcc.dg/tree-ssa/pr117692.c | 17 +++++++++++++++++ gcc/tree.cc | 4 ++-- 4 files changed, 29 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr117692.c (limited to 'gcc') diff --git a/gcc/match.pd b/gcc/match.pd index ad35427..8606b85 100644 --- a/gcc/match.pd +++ b/gcc/match.pd @@ -5577,7 +5577,15 @@ DEFINE_INT_AND_FLOAT_ROUND_FN (RINT) && TREE_CODE (@3) == INTEGER_CST && (mul = wi::mul (wi::to_wide (@2), wi::to_wide (@3), TYPE_SIGN (type), &overflow), - !overflow)) + !overflow) + && (TYPE_UNSIGNED (type) + /* Not using unsigned arithmetics is unsafe if factor + isn't 1 and if for op plus @0 and @2 could have different + sign or for op minus @0 and @2 could have the same sign. */ + || known_eq (factor, 1) + || (get_range_pos_neg (@0) + | (((op == PLUS_EXPR) ^ (tree_int_cst_sgn (@2) < 0)) + ? 1 : 2)) != 3)) (op (mult @0 { wide_int_to_tree (type, factor); }) { wide_int_to_tree (type, mul); }) (with { tree utype = unsigned_type_for (type); } diff --git a/gcc/testsuite/gcc.dg/tree-ssa/mulexactdiv-5.c b/gcc/testsuite/gcc.dg/tree-ssa/mulexactdiv-5.c index 37cd676..af92f65 100644 --- a/gcc/testsuite/gcc.dg/tree-ssa/mulexactdiv-5.c +++ b/gcc/testsuite/gcc.dg/tree-ssa/mulexactdiv-5.c @@ -18,7 +18,7 @@ TEST_CMP (f4, 8, 4, 200) /* { dg-final { scan-tree-dump-not {<[a-z]*_div_expr,} "optimized" } } */ /* { dg-final { scan-tree-dump-not {