On Fri, 25 Feb 2022, Richard Biener wrote:
> This fixes a long-standing issue in PRE where we track valueized
> expressions in our expression sets that we use for PHI translation,
> code insertion but also feed into match-and-simplify via
> vn_nary_simplify. But that's not what is expected from vn_nary_simplify
> or match-and-simplify which assume we are simplifying with operands
> available at the point of the expression so they can use contextual
> information on the SSA names like ranges. While the VN side was
> updated to ensure this with the rewrite to RPO VN, thereby removing
> all workarounds that nullified such contextual info on all SSA names,
> the PRE side still suffers from this.
>
> The following patch tries to apply minimal surgery at this point
> and makes PRE track un-valueized expressions in the expression sets
> but only for the NARY kind (both NAME and CONSTANT do not suffer
> from this issue), leaving the REFERENCE kind alone. The REFERENCE
> kind is important when trying to remove the workarounds still in
> place in compute_avail for code hoisting, but that's a separate issue
> and we have a working workaround in place.
>
> Doing this comes at the cost of duplicating the VN IL on the PRE side
> for NARY and eventually some extra overhead for translated expressions
> that is difficult to assess.
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu.
I've attached the wrong version - the following is the cleaned up
and minimal variant I tested and will push.
Richard.
2022-02-25 Richard Biener <rguent...@suse.de>
PR tree-optimization/103037
* tree-ssa-sccvn.h (alloc_vn_nary_op_noinit): Declare.
(vn_nary_length_from_stmt): Likewise.
(init_vn_nary_op_from_stmt): Likewise.
(vn_nary_op_compute_hash): Likewise.
* tree-ssa-sccvn.cc (alloc_vn_nary_op_noinit): Export.
(vn_nary_length_from_stmt): Likewise.
(init_vn_nary_op_from_stmt): Likewise.
(vn_nary_op_compute_hash): Likewise.
* tree-ssa-pre.cc (pre_expr_obstack): New obstack.
(get_or_alloc_expr_for_nary): Pass in the value-id to use,
(re-)compute the hash value and if the expression is not
found allocate it from pre_expr_obstack.
(phi_translate_1): Do not insert the NARY found in the
VN tables but build a PRE expression from the valueized
NARY with the value-id we eventually found.
(find_or_generate_expression): Assert we have an entry
for constant values.
(compute_avail): Insert not valueized expressions into
EXP_GEN using the value-id from the VN tables.
(init_pre): Allocate pre_expr_obstack.
(fini_pre): Free pre_expr_obstack.
* gcc.dg/torture/pr103037.c: New testcase.
---
gcc/testsuite/gcc.dg/torture/pr103037.c | 23 +++++++++++
gcc/tree-ssa-pre.cc | 52 ++++++++++++++++++-------
gcc/tree-ssa-sccvn.cc | 11 ++----
gcc/tree-ssa-sccvn.h | 4 ++
4 files changed, 68 insertions(+), 22 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/torture/pr103037.c
diff --git a/gcc/testsuite/gcc.dg/torture/pr103037.c
b/gcc/testsuite/gcc.dg/torture/pr103037.c
new file mode 100644
index 00000000000..8b3bb1e4c8b
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr103037.c
@@ -0,0 +1,23 @@
+/* { dg-do run } */
+
+static inline const unsigned short *
+min(unsigned short *d, const unsigned short *e)
+{
+ return *e < *d ? e : d;
+}
+
+unsigned short __attribute__((noipa))
+test(unsigned short arr, unsigned short val)
+{
+ unsigned short tem = 1;
+ unsigned short tem2 = *min(&arr, &tem);
+ return tem2 / (arr ? arr : val);
+}
+
+int
+main()
+{
+ if (test (2, 2) != 0)
+ __builtin_abort();
+ return 0;
+}
diff --git a/gcc/tree-ssa-pre.cc b/gcc/tree-ssa-pre.cc
index 7c2a52e1850..d6c83a72dd8 100644
--- a/gcc/tree-ssa-pre.cc
+++ b/gcc/tree-ssa-pre.cc
@@ -323,6 +323,7 @@ static unsigned int next_expression_id;
static vec<pre_expr> expressions;
static hash_table<pre_expr_d> *expression_to_id;
static vec<unsigned> name_to_id;
+static obstack pre_expr_obstack;
/* Allocate an expression id for EXPR. */
@@ -430,18 +431,23 @@ get_or_alloc_expr_for_name (tree name)
return result;
}
-/* Given an NARY, get or create a pre_expr to represent it. */
+/* Given an NARY, get or create a pre_expr to represent it. Assign
+ VALUE_ID to it or allocate a new value-id if it is zero. Record
+ LOC as the original location of the expression. */
static pre_expr
-get_or_alloc_expr_for_nary (vn_nary_op_t nary,
+get_or_alloc_expr_for_nary (vn_nary_op_t nary, unsigned value_id,
location_t loc = UNKNOWN_LOCATION)
{
struct pre_expr_d expr;
pre_expr result;
unsigned int result_id;
+ gcc_assert (value_id == 0 || !value_id_constant_p (value_id));
+
expr.kind = NARY;
expr.id = 0;
+ nary->hashcode = vn_nary_op_compute_hash (nary);
PRE_EXPR_NARY (&expr) = nary;
result_id = lookup_expression_id (&expr);
if (result_id != 0)
@@ -450,8 +456,10 @@ get_or_alloc_expr_for_nary (vn_nary_op_t nary,
result = pre_expr_pool.allocate ();
result->kind = NARY;
result->loc = loc;
- result->value_id = nary->value_id;
- PRE_EXPR_NARY (result) = nary;
+ result->value_id = value_id ? value_id : get_next_value_id ();
+ PRE_EXPR_NARY (result)
+ = alloc_vn_nary_op_noinit (nary->length, &pre_expr_obstack);
+ memcpy (PRE_EXPR_NARY (result), nary, sizeof_vn_nary_op (nary->length));
alloc_expression_id (result);
return result;
}
@@ -1517,15 +1525,10 @@ phi_translate_1 (bitmap_set_t dest,
return get_or_alloc_expr_for_constant (result);
if (!nary || nary->predicated_values)
- {
- new_val_id = get_next_value_id ();
- nary = vn_nary_op_insert_pieces (newnary->length,
- newnary->opcode,
- newnary->type,
- &newnary->op[0],
- result, new_val_id);
- }
- expr = get_or_alloc_expr_for_nary (nary, expr_loc);
+ new_val_id = 0;
+ else
+ new_val_id = nary->value_id;
+ expr = get_or_alloc_expr_for_nary (newnary, new_val_id, expr_loc);
add_to_value (get_expr_value_id (expr), expr);
}
return expr;
@@ -2789,6 +2792,7 @@ find_or_generate_expression (basic_block block, tree op,
gimple_seq *stmts)
/* Defer. */
return NULL_TREE;
}
+ gcc_assert (!value_id_constant_p (lookfor));
/* It must be a complex expression, so generate it recursively. Note
that this is only necessary to handle gcc.dg/tree-ssa/ssa-pre28.c
@@ -3993,8 +3997,14 @@ compute_avail (function *fun)
FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_DEF)
{
pre_expr e = get_or_alloc_expr_for_name (op);
+ unsigned value_id = get_expr_value_id (e);
+ if (value_id_constant_p (value_id))
+ {
+ get_or_alloc_expr_for_constant (VN_INFO (op)->valnum);
+ continue;
+ }
- add_to_value (get_expr_value_id (e), e);
+ add_to_value (value_id, e);
bitmap_insert_into_set (TMP_GEN (block), e);
bitmap_value_insert_into_set (AVAIL_OUT (block), e);
}
@@ -4073,6 +4083,16 @@ compute_avail (function *fun)
if (!nary || nary->predicated_values)
continue;
+ unsigned value_id = nary->value_id;
+ if (value_id_constant_p (value_id))
+ continue;
+
+ /* Record the un-valueized expression for EXP_GEN. */
+ nary = XALLOCAVAR (struct vn_nary_op_s,
+ sizeof_vn_nary_op
+ (vn_nary_length_from_stmt (stmt)));
+ init_vn_nary_op_from_stmt (nary, as_a <gassign *> (stmt));
+
/* If the NARY traps and there was a preceding
point in the block that might not return avoid
adding the nary to EXP_GEN. */
@@ -4081,7 +4101,7 @@ compute_avail (function *fun)
continue;
result = get_or_alloc_expr_for_nary
- (nary, gimple_location (stmt));
+ (nary, value_id, gimple_location (stmt));
break;
}
@@ -4275,6 +4295,7 @@ init_pre (void)
constant_value_expressions.create (get_max_constant_value_id () + 1);
constant_value_expressions.quick_grow_cleared (get_max_constant_value_id ()
+ 1);
name_to_id.create (0);
+ gcc_obstack_init (&pre_expr_obstack);
inserted_exprs = BITMAP_ALLOC (NULL);
@@ -4312,6 +4333,7 @@ fini_pre ()
delete expression_to_id;
expression_to_id = NULL;
name_to_id.release ();
+ obstack_free (&pre_expr_obstack, NULL);
basic_block bb;
FOR_ALL_BB_FN (bb, cfun)
diff --git a/gcc/tree-ssa-sccvn.cc b/gcc/tree-ssa-sccvn.cc
index 1701861faed..d4d0aba880c 100644
--- a/gcc/tree-ssa-sccvn.cc
+++ b/gcc/tree-ssa-sccvn.cc
@@ -426,11 +426,8 @@ typedef hash_table<vn_ssa_aux_hasher>::iterator
vn_ssa_aux_iterator_type;
static struct obstack vn_ssa_aux_obstack;
static vn_nary_op_t vn_nary_op_insert_stmt (gimple *, tree);
-static unsigned int vn_nary_length_from_stmt (gimple *);
-static vn_nary_op_t alloc_vn_nary_op_noinit (unsigned int, obstack *);
static vn_nary_op_t vn_nary_op_insert_into (vn_nary_op_t,
vn_nary_op_table_type *);
-static void init_vn_nary_op_from_stmt (vn_nary_op_t, gassign *);
static void init_vn_nary_op_from_pieces (vn_nary_op_t, unsigned int,
enum tree_code, tree, tree *);
static tree vn_lookup_simplify_result (gimple_match_op *);
@@ -3854,7 +3851,7 @@ vn_reference_insert_pieces (tree vuse, alias_set_type set,
/* Compute and return the hash value for nary operation VBO1. */
-static hashval_t
+hashval_t
vn_nary_op_compute_hash (const vn_nary_op_t vno1)
{
inchash::hash hstate;
@@ -3927,7 +3924,7 @@ init_vn_nary_op_from_pieces (vn_nary_op_t vno, unsigned
int length,
/* Return the number of operands for a vn_nary ops structure from STMT. */
-static unsigned int
+unsigned int
vn_nary_length_from_stmt (gimple *stmt)
{
switch (gimple_assign_rhs_code (stmt))
@@ -3950,7 +3947,7 @@ vn_nary_length_from_stmt (gimple *stmt)
/* Initialize VNO from STMT. */
-static void
+void
init_vn_nary_op_from_stmt (vn_nary_op_t vno, gassign *stmt)
{
unsigned i;
@@ -4047,7 +4044,7 @@ vn_nary_op_lookup_stmt (gimple *stmt, vn_nary_op_t
*vnresult)
/* Allocate a vn_nary_op_t with LENGTH operands on STACK. */
-static vn_nary_op_t
+vn_nary_op_t
alloc_vn_nary_op_noinit (unsigned int length, struct obstack *stack)
{
return (vn_nary_op_t) obstack_alloc (stack, sizeof_vn_nary_op (length));
diff --git a/gcc/tree-ssa-sccvn.h b/gcc/tree-ssa-sccvn.h
index aeed07039b7..c4e341021e8 100644
--- a/gcc/tree-ssa-sccvn.h
+++ b/gcc/tree-ssa-sccvn.h
@@ -249,6 +249,10 @@ bool has_VN_INFO (tree);
extern vn_ssa_aux_t VN_INFO (tree);
tree vn_get_expr_for (tree);
void scc_vn_restore_ssa_info (void);
+vn_nary_op_t alloc_vn_nary_op_noinit (unsigned int, struct obstack *);
+unsigned int vn_nary_length_from_stmt (gimple *);
+void init_vn_nary_op_from_stmt (vn_nary_op_t, gassign *);
+hashval_t vn_nary_op_compute_hash (const vn_nary_op_t);
tree vn_nary_op_lookup_stmt (gimple *, vn_nary_op_t *);
tree vn_nary_op_lookup_pieces (unsigned int, enum tree_code,
tree, tree *, vn_nary_op_t *);
--
2.34.1
