This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[committed] Fix up cselim regression (PR tree-optimization/52445)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: gcc-patches at gcc dot gnu dot org
- Date: Thu, 1 Mar 2012 15:16:08 +0100
- Subject: [committed] Fix up cselim regression (PR tree-optimization/52445)
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
Hi!
Before MEM_REF merge, this code tracked just SSA_NAME dereferences, which
have all the same access size and offset. MEM_REFs with the same SSA_NAME
can have different sizes and/or offsets from the base SSA_NAME.
This patch teaches the non-trapping discovery code to take size/offset
into account (in a simple way, in theory a wider store or multiple
smaller stores could serve as well). Pre-approved by richi in the PR,
bootstrapped/regtested on x86_64-linux and i686-linux, committed to trunk.
2012-03-01 Jakub Jelinek <jakub@redhat.com>
PR tree-optimization/52445
* tree-ssa-phiopt.c (struct name_to_bb): Remove ssa_name field,
add ssa_name_ver, offset and size fields and change store field
to bool.
(name_to_bb_hash, name_to_bb_eq): Adjust for the above changes.
(add_or_mark_expr): Likewise. Only consider previous stores
with the same size and offset.
(nt_init_block): Only look at gimple_assign_single_p stmts,
doesn't look at rhs2.
* gcc.dg/pr52445.c: New test.
--- gcc/tree-ssa-phiopt.c.jj 2011-11-10 18:09:12.000000000 +0100
+++ gcc/tree-ssa-phiopt.c 2012-03-01 12:07:03.925652523 +0100
@@ -1122,9 +1122,10 @@ abs_replacement (basic_block cond_bb, ba
same accesses. */
struct name_to_bb
{
- tree ssa_name;
+ unsigned int ssa_name_ver;
+ bool store;
+ HOST_WIDE_INT offset, size;
basic_block bb;
- unsigned store : 1;
};
/* The hash table for remembering what we've seen. */
@@ -1133,23 +1134,26 @@ static htab_t seen_ssa_names;
/* The set of MEM_REFs which can't trap. */
static struct pointer_set_t *nontrap_set;
-/* The hash function, based on the pointer to the pointer SSA_NAME. */
+/* The hash function. */
static hashval_t
name_to_bb_hash (const void *p)
{
- const_tree n = ((const struct name_to_bb *)p)->ssa_name;
- return htab_hash_pointer (n) ^ ((const struct name_to_bb *)p)->store;
+ const struct name_to_bb *n = (const struct name_to_bb *) p;
+ return n->ssa_name_ver ^ (((hashval_t) n->store) << 31)
+ ^ (n->offset << 6) ^ (n->size << 3);
}
-/* The equality function of *P1 and *P2. SSA_NAMEs are shared, so
- it's enough to simply compare them for equality. */
+/* The equality function of *P1 and *P2. */
static int
name_to_bb_eq (const void *p1, const void *p2)
{
const struct name_to_bb *n1 = (const struct name_to_bb *)p1;
const struct name_to_bb *n2 = (const struct name_to_bb *)p2;
- return n1->ssa_name == n2->ssa_name && n1->store == n2->store;
+ return n1->ssa_name_ver == n2->ssa_name_ver
+ && n1->store == n2->store
+ && n1->offset == n2->offset
+ && n1->size == n2->size;
}
/* We see the expression EXP in basic block BB. If it's an interesting
@@ -1161,8 +1165,12 @@ static void
add_or_mark_expr (basic_block bb, tree exp,
struct pointer_set_t *nontrap, bool store)
{
+ HOST_WIDE_INT size;
+
if (TREE_CODE (exp) == MEM_REF
- && TREE_CODE (TREE_OPERAND (exp, 0)) == SSA_NAME)
+ && TREE_CODE (TREE_OPERAND (exp, 0)) == SSA_NAME
+ && host_integerp (TREE_OPERAND (exp, 1), 0)
+ && (size = int_size_in_bytes (TREE_TYPE (exp))) > 0)
{
tree name = TREE_OPERAND (exp, 0);
struct name_to_bb map;
@@ -1172,9 +1180,12 @@ add_or_mark_expr (basic_block bb, tree e
/* Try to find the last seen MEM_REF through the same
SSA_NAME, which can trap. */
- map.ssa_name = name;
+ map.ssa_name_ver = SSA_NAME_VERSION (name);
map.bb = 0;
map.store = store;
+ map.offset = tree_low_cst (TREE_OPERAND (exp, 1), 0);
+ map.size = size;
+
slot = htab_find_slot (seen_ssa_names, &map, INSERT);
n2bb = (struct name_to_bb *) *slot;
if (n2bb)
@@ -1197,9 +1208,11 @@ add_or_mark_expr (basic_block bb, tree e
else
{
n2bb = XNEW (struct name_to_bb);
- n2bb->ssa_name = name;
+ n2bb->ssa_name_ver = SSA_NAME_VERSION (name);
n2bb->bb = bb;
n2bb->store = store;
+ n2bb->offset = map.offset;
+ n2bb->size = size;
*slot = n2bb;
}
}
@@ -1219,13 +1232,10 @@ nt_init_block (struct dom_walk_data *dat
{
gimple stmt = gsi_stmt (gsi);
- if (is_gimple_assign (stmt))
+ if (gimple_assign_single_p (stmt))
{
add_or_mark_expr (bb, gimple_assign_lhs (stmt), nontrap_set, true);
add_or_mark_expr (bb, gimple_assign_rhs1 (stmt), nontrap_set, false);
- if (get_gimple_rhs_num_ops (gimple_assign_rhs_code (stmt)) > 1)
- add_or_mark_expr (bb, gimple_assign_rhs2 (stmt), nontrap_set,
- false);
}
}
}
--- gcc/testsuite/gcc.dg/pr52445.c.jj 2012-03-01 11:32:41.119917248 +0100
+++ gcc/testsuite/gcc.dg/pr52445.c 2012-03-01 11:32:29.000000000 +0100
@@ -0,0 +1,15 @@
+/* PR tree-optimization/52445 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -ftree-cselim -fdump-tree-cselim" } */
+
+void
+foo (char *buf, unsigned long len)
+{
+ buf[0] = '\n';
+ if (len > 1)
+ buf[1] = '\0'; /* We can't cselim "optimize" this, while
+ buf[0] doesn't trap, buf[1] could. */
+}
+
+/* { dg-final { scan-tree-dump-not "cstore\." "cselim" } } */
+/* { dg-final { cleanup-tree-dump "cselim" } } */
Jakub