Re: [RFA][PATCH 3/4] Trim mem* calls in DSE

[Jeff Law]

On 01/04/2017 07:04 AM, Richard Biener wrote:


        * 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.
It doesn't seem needed anymore.  I'm ripping that out.

+/* 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);
*countp is still a tree, but we know its an INTEGER_CST, so we can 
extract its value trivially.

+}
+
+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 ...
Done.  I'm pretty sure calling into build_ref_for_offset was to handle 
&foo.bar kinds of cases and as you note, we can just re-fold the thing 
as a MEM_REF which simplifies everything a little.

Jeff