| Message ID | 20191125233011.GD10088@tucnak |
|---|---|
| State | New |
| Headers | show |
| Series | Fix phiopt minmax optimization with NULLPTR_TYPE (PR tree-optimization/92644) | expand |
On Tue, 26 Nov 2019, Jakub Jelinek wrote: > Hi! > > The following testcase ICEs, because we assert the only constants with > NULLPTR_TYPE created are 0. > > The fix is to perform the value adjustment of boundary value and EQ/NE > conversion to other comparisons only for scalar integral types, not anything > that happens to be INTEGER_CST (other types are pointers/references or > NULLPTR_TYPE, maybe something else). > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? OK. It just occured to me - what if we'd use the type of the non-constant argument for creating the adjusted constant? Wouldn't that avoid the issue as well? Thanks, Richard. > 2019-11-26 Jakub Jelinek <jakub@redhat.com> > > PR tree-optimization/92644 > * tree-ssa-phiopt.c (minmax_replacement): Add INTEGRAL_TYPE_P check > next to INTEGER_CST checks. > > * g++.dg/opt/pr92644.C: New test. > > --- gcc/tree-ssa-phiopt.c.jj 2019-11-20 09:25:42.552157763 +0100 > +++ gcc/tree-ssa-phiopt.c 2019-11-25 11:38:04.181024587 +0100 > @@ -1381,7 +1381,8 @@ minmax_replacement (basic_block cond_bb, > > /* Turn EQ/NE of extreme values to order comparisons. */ > if ((cmp == NE_EXPR || cmp == EQ_EXPR) > - && TREE_CODE (rhs) == INTEGER_CST) > + && TREE_CODE (rhs) == INTEGER_CST > + && INTEGRAL_TYPE_P (TREE_TYPE (rhs))) > { > if (wi::eq_p (wi::to_wide (rhs), wi::min_value (TREE_TYPE (rhs)))) > { > @@ -1407,7 +1408,8 @@ minmax_replacement (basic_block cond_bb, > larger = rhs; > /* If we have smaller < CST it is equivalent to smaller <= CST-1. > Likewise smaller <= CST is equivalent to smaller < CST+1. */ > - if (TREE_CODE (larger) == INTEGER_CST) > + if (TREE_CODE (larger) == INTEGER_CST > + && INTEGRAL_TYPE_P (TREE_TYPE (larger))) > { > if (cmp == LT_EXPR) > { > @@ -1435,7 +1437,8 @@ minmax_replacement (basic_block cond_bb, > larger = gimple_cond_lhs (cond); > /* If we have larger > CST it is equivalent to larger >= CST+1. > Likewise larger >= CST is equivalent to larger > CST-1. */ > - if (TREE_CODE (smaller) == INTEGER_CST) > + if (TREE_CODE (smaller) == INTEGER_CST > + && INTEGRAL_TYPE_P (TREE_TYPE (smaller))) > { > wi::overflow_type overflow; > if (cmp == GT_EXPR) > --- gcc/testsuite/g++.dg/opt/pr92644.C.jj 2019-11-25 11:38:34.599555557 +0100 > +++ gcc/testsuite/g++.dg/opt/pr92644.C 2019-11-25 11:39:59.630244452 +0100 > @@ -0,0 +1,6 @@ > +// PR tree-optimization/92644 > +// { dg-do compile { target c++14 } } > +// { dg-options "-O2 -fno-early-inlining" } > + > +inline auto foo () { return nullptr; } > +int bar () { return foo () ? 1 : 0; } > > Jakub > >
On Tue, Nov 26, 2019 at 09:07:11AM +0100, Richard Biener wrote: > OK. > > It just occured to me - what if we'd use the type of the non-constant > argument for creating the adjusted constant? Wouldn't that avoid > the issue as well? No, the type type of the non-constant argument is NULLPTR_TYPE too. I guess we could have some VRP optimization that range of NULLPTR_TYPE is [0, 0], but I think we don't really perform VRP on non-INTEGRAL/POINTER_TYPE_P, still, phiopt couldn't rely on that. What could work is checking the PHI args earlier if at least one of them matches the non-constant one, but I think that would break the if (a <= u) b = MAX (a, d); x = PHI <b, u> opt. In particular, on this exact testcase the EQ/NE transformation to non-equality comparison is useless, as it will not really match. But say with pointers it could hit even if it is not useless. Jakub
--- gcc/tree-ssa-phiopt.c.jj 2019-11-20 09:25:42.552157763 +0100 +++ gcc/tree-ssa-phiopt.c 2019-11-25 11:38:04.181024587 +0100 @@ -1381,7 +1381,8 @@ minmax_replacement (basic_block cond_bb, /* Turn EQ/NE of extreme values to order comparisons. */ if ((cmp == NE_EXPR || cmp == EQ_EXPR) - && TREE_CODE (rhs) == INTEGER_CST) + && TREE_CODE (rhs) == INTEGER_CST + && INTEGRAL_TYPE_P (TREE_TYPE (rhs))) { if (wi::eq_p (wi::to_wide (rhs), wi::min_value (TREE_TYPE (rhs)))) { @@ -1407,7 +1408,8 @@ minmax_replacement (basic_block cond_bb, larger = rhs; /* If we have smaller < CST it is equivalent to smaller <= CST-1. Likewise smaller <= CST is equivalent to smaller < CST+1. */ - if (TREE_CODE (larger) == INTEGER_CST) + if (TREE_CODE (larger) == INTEGER_CST + && INTEGRAL_TYPE_P (TREE_TYPE (larger))) { if (cmp == LT_EXPR) { @@ -1435,7 +1437,8 @@ minmax_replacement (basic_block cond_bb, larger = gimple_cond_lhs (cond); /* If we have larger > CST it is equivalent to larger >= CST+1. Likewise larger >= CST is equivalent to larger > CST-1. */ - if (TREE_CODE (smaller) == INTEGER_CST) + if (TREE_CODE (smaller) == INTEGER_CST + && INTEGRAL_TYPE_P (TREE_TYPE (smaller))) { wi::overflow_type overflow; if (cmp == GT_EXPR) --- gcc/testsuite/g++.dg/opt/pr92644.C.jj 2019-11-25 11:38:34.599555557 +0100 +++ gcc/testsuite/g++.dg/opt/pr92644.C 2019-11-25 11:39:59.630244452 +0100 @@ -0,0 +1,6 @@ +// PR tree-optimization/92644 +// { dg-do compile { target c++14 } } +// { dg-options "-O2 -fno-early-inlining" } + +inline auto foo () { return nullptr; } +int bar () { return foo () ? 1 : 0; }