From e5ab5ae072f7555eb8019c16cb394c18b88b5dc2 Mon Sep 17 00:00:00 2001 From: Richard Biener Date: Tue, 8 Jan 2019 13:05:47 +0000 Subject: [PATCH] re PR middle-end/86554 (Incorrect code generation with signed/unsigned comparison) 2019-01-08 Richard Biener PR tree-optimization/86554 * tree-ssa-sccvn.c (eliminate_dom_walker, rpo_elim, rpo_avail): Move earlier. (visit_nary_op): When value-numbering to expressions with different overflow behavior make sure there's an available expression on the path. * gcc.dg/torture/pr86554-1.c: New testcase. * gcc.dg/torture/pr86554-2.c: Likewise. From-SVN: r267725 --- gcc/ChangeLog | 9 ++ gcc/testsuite/ChangeLog | 6 + gcc/testsuite/gcc.dg/torture/pr86554-1.c | 35 +++++ gcc/testsuite/gcc.dg/torture/pr86554-2.c | 49 +++++++ gcc/tree-ssa-sccvn.c | 170 ++++++++++++----------- 5 files changed, 188 insertions(+), 81 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr86554-1.c create mode 100644 gcc/testsuite/gcc.dg/torture/pr86554-2.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 95208f3f058..bc313e171af 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,12 @@ +2019-01-08 Richard Biener + + PR tree-optimization/86554 + * tree-ssa-sccvn.c (eliminate_dom_walker, rpo_elim, + rpo_avail): Move earlier. + (visit_nary_op): When value-numbering to expressions + with different overflow behavior make sure there's an + available expression on the path. + 2019-01-08 Sam Tebbs * config/aarch64/aarch64.c (BRANCH_PROTECT_STR_MAX, diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 84c3cd88857..98a851c072c 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2019-01-08 Richard Biener + + PR tree-optimization/86554 + * gcc.dg/torture/pr86554-1.c: New testcase. + * gcc.dg/torture/pr86554-2.c: Likewise. + 2019-01-08 Paolo Carlini * g++.dg/diagnostic/thread1.C: Tweak expected error #line 13 to diff --git a/gcc/testsuite/gcc.dg/torture/pr86554-1.c b/gcc/testsuite/gcc.dg/torture/pr86554-1.c new file mode 100644 index 00000000000..64f851e896e --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr86554-1.c @@ -0,0 +1,35 @@ +/* { dg-do run } */ + +struct foo +{ + unsigned x; +}; +typedef struct foo foo; + +static inline int zot(foo *f) +{ + int ret; + + if (f->x > 0x7FFFFFFF) + ret = (int)(f->x - 0x7FFFFFFF); + else + ret = (int)f->x - 0x7FFFFFFF; + return ret; +} + +void __attribute__((noinline,noclone)) bar(foo *f) +{ + int ret = zot(f); + volatile int x = ret; + if (ret < 1) + __builtin_abort (); +} + +int main() +{ + foo f; + f.x = 0x800003f8; + + bar(&f); + return 0; +} diff --git a/gcc/testsuite/gcc.dg/torture/pr86554-2.c b/gcc/testsuite/gcc.dg/torture/pr86554-2.c new file mode 100644 index 00000000000..9e57a9ca725 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr86554-2.c @@ -0,0 +1,49 @@ +/* { dg-do run } */ +/* { dg-require-effective-target int32plus } */ + +struct s { __INT64_TYPE__ e; }; + +static void f (struct s *ps) +{ + volatile __INT64_TYPE__ m = 9223372036854775807; + const char *str = "11E"; + int r; + __INT64_TYPE__ sum; + + ps->e = 0; + + for (;;) + { + if (*str++ != '1') + break; + ps->e ++; + } + + r = 1; + sum = m; + + if (sum >= 0 && ps->e >= 0) + { + __UINT64_TYPE__ uc; + uc = (__UINT64_TYPE__) sum + (__UINT64_TYPE__) ps->e; + if (uc > 9223372036854775807) + r = 2; + else + sum = 17; + } + else + sum = sum + ps->e; + + if (sum != 9223372036854775807) + __builtin_abort (); + if (r != 2) + __builtin_abort (); + ps->e = sum; +} + +int main (void) +{ + struct s s; + f (&s); + return 0; +} diff --git a/gcc/tree-ssa-sccvn.c b/gcc/tree-ssa-sccvn.c index 2cebb6b37dc..ff54a66534e 100644 --- a/gcc/tree-ssa-sccvn.c +++ b/gcc/tree-ssa-sccvn.c @@ -1865,6 +1865,86 @@ vn_nary_simplify (vn_nary_op_t nary) return vn_nary_build_or_lookup_1 (&op, false); } +/* Elimination engine. */ + +class eliminate_dom_walker : public dom_walker +{ +public: + eliminate_dom_walker (cdi_direction, bitmap); + ~eliminate_dom_walker (); + + virtual edge before_dom_children (basic_block); + virtual void after_dom_children (basic_block); + + virtual tree eliminate_avail (basic_block, tree op); + virtual void eliminate_push_avail (basic_block, tree op); + tree eliminate_insert (basic_block, gimple_stmt_iterator *gsi, tree val); + + void eliminate_stmt (basic_block, gimple_stmt_iterator *); + + unsigned eliminate_cleanup (bool region_p = false); + + bool do_pre; + unsigned int el_todo; + unsigned int eliminations; + unsigned int insertions; + + /* SSA names that had their defs inserted by PRE if do_pre. */ + bitmap inserted_exprs; + + /* Blocks with statements that have had their EH properties changed. */ + bitmap need_eh_cleanup; + + /* Blocks with statements that have had their AB properties changed. */ + bitmap need_ab_cleanup; + + /* Local state for the eliminate domwalk. */ + auto_vec to_remove; + auto_vec to_fixup; + auto_vec avail; + auto_vec avail_stack; +}; + +/* Adaptor to the elimination engine using RPO availability. */ + +class rpo_elim : public eliminate_dom_walker +{ +public: + rpo_elim(basic_block entry_) + : eliminate_dom_walker (CDI_DOMINATORS, NULL), entry (entry_) {} + ~rpo_elim(); + + virtual tree eliminate_avail (basic_block, tree op); + + virtual void eliminate_push_avail (basic_block, tree); + + basic_block entry; + /* Instead of having a local availability lattice for each + basic-block and availability at X defined as union of + the local availabilities at X and its dominators we're + turning this upside down and track availability per + value given values are usually made available at very + few points (at least one). + So we have a value -> vec map where + LOCATION is specifying the basic-block LEADER is made + available for VALUE. We push to this vector in RPO + order thus for iteration we can simply pop the last + entries. + LOCATION is the basic-block index and LEADER is its + SSA name version. */ + /* ??? We'd like to use auto_vec here with embedded storage + but that doesn't play well until we can provide move + constructors and use std::move on hash-table expansion. + So for now this is a bit more expensive than necessary. + We eventually want to switch to a chaining scheme like + for hashtable entries for unwinding which would make + making the vector part of the vn_ssa_aux structure possible. */ + typedef hash_map > > rpo_avail_t; + rpo_avail_t m_rpo_avail; +}; + +/* Global RPO state for access from hooks. */ +static rpo_elim *rpo_avail; basic_block vn_context_bb; /* Callback for walk_non_aliased_vuses. Tries to perform a lookup @@ -3843,7 +3923,15 @@ visit_nary_op (tree lhs, gassign *stmt) ops[0] = vn_nary_op_lookup_pieces (2, gimple_assign_rhs_code (def), type, ops, NULL); /* We have wider operation available. */ - if (ops[0]) + if (ops[0] + /* If the leader is a wrapping operation we can + insert it for code hoisting w/o introducing + undefined overflow. If it is not it has to + be available. See PR86554. */ + && (TYPE_OVERFLOW_WRAPS (TREE_TYPE (ops[0])) + || (rpo_avail && vn_context_bb + && rpo_avail->eliminate_avail (vn_context_bb, + ops[0])))) { unsigned lhs_prec = TYPE_PRECISION (type); unsigned rhs_prec = TYPE_PRECISION (TREE_TYPE (rhs1)); @@ -4654,45 +4742,6 @@ vn_nary_may_trap (vn_nary_op_t nary) return false; } - -class eliminate_dom_walker : public dom_walker -{ -public: - eliminate_dom_walker (cdi_direction, bitmap); - ~eliminate_dom_walker (); - - virtual edge before_dom_children (basic_block); - virtual void after_dom_children (basic_block); - - virtual tree eliminate_avail (basic_block, tree op); - virtual void eliminate_push_avail (basic_block, tree op); - tree eliminate_insert (basic_block, gimple_stmt_iterator *gsi, tree val); - - void eliminate_stmt (basic_block, gimple_stmt_iterator *); - - unsigned eliminate_cleanup (bool region_p = false); - - bool do_pre; - unsigned int el_todo; - unsigned int eliminations; - unsigned int insertions; - - /* SSA names that had their defs inserted by PRE if do_pre. */ - bitmap inserted_exprs; - - /* Blocks with statements that have had their EH properties changed. */ - bitmap need_eh_cleanup; - - /* Blocks with statements that have had their AB properties changed. */ - bitmap need_ab_cleanup; - - /* Local state for the eliminate domwalk. */ - auto_vec to_remove; - auto_vec to_fixup; - auto_vec avail; - auto_vec avail_stack; -}; - eliminate_dom_walker::eliminate_dom_walker (cdi_direction direction, bitmap inserted_exprs_) : dom_walker (direction), do_pre (inserted_exprs_ != NULL), @@ -5648,47 +5697,6 @@ free_rpo_vn (void) BITMAP_FREE (constant_value_ids); } -/* Adaptor to the elimination engine using RPO availability. */ - -class rpo_elim : public eliminate_dom_walker -{ -public: - rpo_elim(basic_block entry_) - : eliminate_dom_walker (CDI_DOMINATORS, NULL), entry (entry_) {} - ~rpo_elim(); - - virtual tree eliminate_avail (basic_block, tree op); - - virtual void eliminate_push_avail (basic_block, tree); - - basic_block entry; - /* Instead of having a local availability lattice for each - basic-block and availability at X defined as union of - the local availabilities at X and its dominators we're - turning this upside down and track availability per - value given values are usually made available at very - few points (at least one). - So we have a value -> vec map where - LOCATION is specifying the basic-block LEADER is made - available for VALUE. We push to this vector in RPO - order thus for iteration we can simply pop the last - entries. - LOCATION is the basic-block index and LEADER is its - SSA name version. */ - /* ??? We'd like to use auto_vec here with embedded storage - but that doesn't play well until we can provide move - constructors and use std::move on hash-table expansion. - So for now this is a bit more expensive than necessary. - We eventually want to switch to a chaining scheme like - for hashtable entries for unwinding which would make - making the vector part of the vn_ssa_aux structure possible. */ - typedef hash_map > > rpo_avail_t; - rpo_avail_t m_rpo_avail; -}; - -/* Global RPO state for access from hooks. */ -static rpo_elim *rpo_avail; - /* Hook for maybe_push_res_to_seq, lookup the expression in the VN tables. */ static tree -- 2.30.2