On 01/04/2017 07:04 AM, Richard Biener wrote:
Didn't see a re-post of this one so reviewing the old.
Didn't figure mem* trimming was suitable for gcc-7 as I couldn't justify
it as a bugfix, so I didn't ping it.
I don't think it changed materially. All your comments are still
applicable to the version in my tree.
* tree-ssa-dse.c (need_ssa_update): New file scoped boolean.
(decrement_count): New function.
(increment_start_addr, trim_memstar_call): Likewise.
(trim_partially_dead_store): Call trim_memstar_call.
(pass_dse::execute): Initialize need_ssa_update. If set, then
return TODO_ssa_update.
* gcc.dg/tree-ssa/ssa-dse-25.c: New test.
diff --git a/gcc/tree-ssa-dse.c b/gcc/tree-ssa-dse.c
index 1482c7f..b21b9b5 100644
--- a/gcc/tree-ssa-dse.c
+++ b/gcc/tree-ssa-dse.c
@@ -79,6 +80,10 @@ static bitmap need_eh_cleanup;
It is always safe to return FALSE. But typically better optimziation
can be achieved by analyzing more statements. */
+/* If trimming stores requires insertion of new statements, then we
+ will need an SSA update. */
+static bool need_ssa_update;
+
huh? You set this to true after inserting a POINTER_PLUS_EXPR, I don't see
how you need an SSA update for this.
I'll go back and re-investigate. I could easily have goof'd the
in-place update and be papering over that with the ssa update.
static bool
initialize_ao_ref_for_dse (gimple *stmt, ao_ref *write)
{
@@ -309,6 +314,113 @@ trim_constructor_store (bitmap orig, bitmap live,
gimple *stmt)
}
}
+/* STMT is a memcpy, memmove or memset. Decrement the number of bytes
+ copied/set by DECREMENT. */
+static void
+decrement_count (gimple *stmt, int decrement)
+{
+ tree *countp = gimple_call_arg_ptr (stmt, 2);
+ gcc_assert (TREE_CODE (*countp) == INTEGER_CST);
+ tree x = fold_build2 (MINUS_EXPR, TREE_TYPE (*countp), *countp,
+ build_int_cst (TREE_TYPE (*countp), decrement));
+ *countp = x;
thanks to wide-int the following should work
*countp = wide_int_to_tree (TREE_TYPE (*countp), *countp - decrement);
Sweet. I like that much better.
(if not please use int_const_binop rather than fold_build2 here and
below as well)
+}
+
+static void
+increment_start_addr (gimple *stmt ATTRIBUTE_UNUSED, tree *where, int
increment)
+{
+ /* If the address wasn't initially a MEM_REF, make it a MEM_REF. */
+ if (TREE_CODE (*where) == ADDR_EXPR
+ && TREE_CODE (TREE_OPERAND (*where, 0)) != MEM_REF)
+ {
+ tree t = TREE_OPERAND (*where, 0);
+ t = build_ref_for_offset (EXPR_LOCATION (t), t,
+ increment * BITS_PER_UNIT, false,
+ ptr_type_node, NULL, false);
please don't use build_ref_for_offset for this. Simply only handle the SSA_NAME
case here and below ...
I think build_ref_for_offset was what spurred the tree-sra.h inclusion.
IIRC I was seeing a goodly number of cases where the argument wasn't a
MEM_REF or SSA_NAME at this point. But I'll double-check.
If we don't need build_ref_for_offset, do you still want me to pull its
prototype into the new tree-sra.h, or just leave it as-is?
+ *where = build_fold_addr_expr (t);
+ return;
+ }
+ else if (TREE_CODE (*where) == SSA_NAME)
+ {
+ tree tem = make_ssa_name (TREE_TYPE (*where));
+ gassign *newop
+ = gimple_build_assign (tem, POINTER_PLUS_EXPR, *where,
+ build_int_cst (sizetype, increment));
+ gimple_stmt_iterator gsi = gsi_for_stmt (stmt);
+ gsi_insert_before (&gsi, newop, GSI_SAME_STMT);
+ need_ssa_update = true;
+ *where = tem;
+ update_stmt (gsi_stmt (gsi));
+ return;
+ }
+
+ /* We can just adjust the offset in the MEM_REF expression. */
+ tree x1 = TREE_OPERAND (TREE_OPERAND (*where, 0), 1);
+ tree x = fold_build2 (PLUS_EXPR, TREE_TYPE (x1), x1,
+ build_int_cst (TREE_TYPE (x1), increment));
+ TREE_OPERAND (TREE_OPERAND (*where, 0), 1) = x;
...
re-fold the thing as MEM_REF which will do all the magic for you:
*where = build_fold_addr_expr (fold_build2 (MEM_REF, char_type_node,
*where, build_int_cst (ptr_type_node, increment)));
that handles &MEM[] and &foo.bar just fine and avoids adding magic here.
And that (&foo.bar) is likely what I was looking to handle with the
first if clause above where I called build_ref_for_offset.
Otherwise looks ok. I think I'd like to see this in GCC 7 given it's
so much similar to the constructor pruning.
OK. I'll sort through the issues noted above and get this one reposted
as well.
jeff