| Message ID | 1752ce19-956b-a055-2585-a6b0e2827572@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | PR tree-optimization/107985 - Ensure arguments to range-op handler are supported. | expand |
On Wed, Dec 7, 2022 at 5:45 PM Andrew MacLeod via Gcc-patches <gcc-patches@gcc.gnu.org> wrote: > > THis patch invalidates a range-op handler object if an operand type in > the statement is not supported. > > This also triggered a check in stmt dependency resolution which assumed > there must be a valid handler for any stmt with an appropriate LHS > type... which is a false assumption. > > This should do for now, but long term I will rework the dispatch code to > ensure it matches the specifically supported patterns of operands. This > will make the handler creation a little slower, but speed up the actual > dispatch, especially as we add new range types next release. Its also > much more invasive... too much for this release I think. > > bootstraps on x86_64-pc-linux-gnu with no regressions. OK? + if (!Value_Range::supports_type_p (TREE_TYPE (m_op1)) || + !Value_Range::supports_type_p (TREE_TYPE (m_op2))) The ||s go to the next line. Since in a GIMPLE_COND both operand types are compatible it's enough to check one of them. Likewise for the GIMPLE_ASSIGN case I think - I don't know of any binary operator that has operands that would not be both compatible or not compatible (but it's less clear-cut here). Otherwise looks straight forward. Thanks, Richard. > Andrew >
On 12/7/22 12:26, Richard Biener wrote: > On Wed, Dec 7, 2022 at 5:45 PM Andrew MacLeod via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >> THis patch invalidates a range-op handler object if an operand type in >> the statement is not supported. >> >> This also triggered a check in stmt dependency resolution which assumed >> there must be a valid handler for any stmt with an appropriate LHS >> type... which is a false assumption. >> >> This should do for now, but long term I will rework the dispatch code to >> ensure it matches the specifically supported patterns of operands. This >> will make the handler creation a little slower, but speed up the actual >> dispatch, especially as we add new range types next release. Its also >> much more invasive... too much for this release I think. >> >> bootstraps on x86_64-pc-linux-gnu with no regressions. OK? > + if (!Value_Range::supports_type_p (TREE_TYPE (m_op1)) || > + !Value_Range::supports_type_p (TREE_TYPE (m_op2))) > > The ||s go to the next line. Since in a GIMPLE_COND both operand types > are compatible it's enough to check one of them. > > Likewise for the GIMPLE_ASSIGN case I think - I don't know of any > binary operator that has operands that would not be both compatible > or not compatible (but it's less clear-cut here). > Doh. Checked this in: Andrew
From 966076046e5687937eeac61df762f89178aa17c7 Mon Sep 17 00:00:00 2001 From: Andrew MacLeod <amacleod@redhat.com> Date: Tue, 6 Dec 2022 10:41:29 -0500 Subject: [PATCH 1/2] Ensure arguments to range-op handler are supported. PR tree-optimization/107985 gcc/ * gimple-range-op.cc (gimple_range_op_handler::gimple_range_op_handler): Check if type of the operands is supported. * gimple-range.cc (gimple_ranger::prefill_stmt_dependencies): Do not assert if here is no range-op handler. gcc/testsuite/ * g++.dg/pr107985.C: New. --- gcc/gimple-range-op.cc | 6 ++++++ gcc/gimple-range.cc | 24 +++++++++++++----------- gcc/testsuite/g++.dg/pr107985.C | 18 ++++++++++++++++++ 3 files changed, 37 insertions(+), 11 deletions(-) create mode 100644 gcc/testsuite/g++.dg/pr107985.C diff --git a/gcc/gimple-range-op.cc b/gcc/gimple-range-op.cc index 7764166d5fb..c36c49ac1da 100644 --- a/gcc/gimple-range-op.cc +++ b/gcc/gimple-range-op.cc @@ -148,6 +148,9 @@ gimple_range_op_handler::gimple_range_op_handler (gimple *s) case GIMPLE_COND: m_op1 = gimple_cond_lhs (m_stmt); m_op2 = gimple_cond_rhs (m_stmt); + if (!Value_Range::supports_type_p (TREE_TYPE (m_op1)) || + !Value_Range::supports_type_p (TREE_TYPE (m_op2))) + m_valid = false; return; case GIMPLE_ASSIGN: m_op1 = gimple_range_base_of_assignment (m_stmt); @@ -164,6 +167,9 @@ gimple_range_op_handler::gimple_range_op_handler (gimple *s) } if (gimple_num_ops (m_stmt) >= 3) m_op2 = gimple_assign_rhs2 (m_stmt); + if ((m_op1 && !Value_Range::supports_type_p (TREE_TYPE (m_op1))) || + (m_op2 && !Value_Range::supports_type_p (TREE_TYPE (m_op2)))) + m_valid = false; return; default: gcc_unreachable (); diff --git a/gcc/gimple-range.cc b/gcc/gimple-range.cc index ecd6039e0fd..8c055826e17 100644 --- a/gcc/gimple-range.cc +++ b/gcc/gimple-range.cc @@ -422,18 +422,20 @@ gimple_ranger::prefill_stmt_dependencies (tree ssa) else { gimple_range_op_handler handler (stmt); - gcc_checking_assert (handler); - tree op = handler.operand2 (); - if (op) + if (handler) { - Value_Range r (TREE_TYPE (op)); - prefill_name (r, op); - } - op = handler.operand1 (); - if (op) - { - Value_Range r (TREE_TYPE (op)); - prefill_name (r, op); + tree op = handler.operand2 (); + if (op) + { + Value_Range r (TREE_TYPE (op)); + prefill_name (r, op); + } + op = handler.operand1 (); + if (op) + { + Value_Range r (TREE_TYPE (op)); + prefill_name (r, op); + } } } } diff --git a/gcc/testsuite/g++.dg/pr107985.C b/gcc/testsuite/g++.dg/pr107985.C new file mode 100644 index 00000000000..8d244b54efb --- /dev/null +++ b/gcc/testsuite/g++.dg/pr107985.C @@ -0,0 +1,18 @@ +/* { dg-do compile } */ +/* { dg-options "-O1 -ftree-vrp -fno-tree-ccp -fno-tree-forwprop -fno-tree-fre" } */ + +struct B { + int f; +}; + +struct D : public B { +}; + +void foo() { + D d; + d.f = 7; + + int B::* pfb = &B::f; + int D::* pfd = pfb; + int v = d.*pfd; +} -- 2.38.1