https://gcc.gnu.org/bugzilla/show_bug.cgi?id=121726

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rguenth at gcc dot gnu.org

--- Comment #12 from Richard Biener <rguenth at gcc dot gnu.org> ---
Ouch.  So build_ref_for_offset does not receive enough info and what we record
as 'base' in an access no longer carries all relevant info (only ->expr does). 
There's many calls to build_ref_for_offset, the particular case is through
build_ref_for_model.  The core issue is we strip both address-space and alias
info from MEM_REFs of decls eventually via get_ref_base_and_extent (that's
intended).

It looks like build_ref_for_offset would DTRT iff we'd passed down the core
MEM_REF of ->expr, but of course we have to adjust for the possibly now
again embedded offset therein.

diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc
index 151f6005ff3..c8849e7af93 100644
--- a/gcc/tree-sra.cc
+++ b/gcc/tree-sra.cc
@@ -2041,8 +2041,16 @@ build_ref_for_model (location_t loc, tree base,
HOST_WIDE_INT offset,
          && (res = build_reconstructed_reference (loc, base, model)))
        return res;
       else
-       return build_ref_for_offset (loc, base, offset, model->reverse,
-                                    model->type, gsi, insert_after);
+       {
+         gcc_assert (base == model->base);
+         tree orig_base = model->expr;
+         while (handled_component_p (orig_base))
+           orig_base = TREE_OPERAND (orig_base, 0);
+         if (TREE_CODE (orig_base) == MEM_REF)
+           offset -= TREE_INT_CST_LOW (TREE_OPERAND (orig_base, 1));
+         return build_ref_for_offset (loc, orig_base, offset, model->reverse,
+                                      model->type, gsi, insert_after);
+       }
     }
 }


fixes the testcase, but the assert shows where it will break - so the
caller is responsible to pass down the appropriate base instead.  In
this case this is

#1  0x0000000001c06b83 in generate_subtree_copies (access=0xa1b5ee0, 
    agg=<var_decl 0x7ffff20f1688 ctx>, top_offset=0, start_offset=0, 
    chunk_size=0, gsi=0x7fffffffd730, write=false, insert_after=false, 
    loc=4611686018427424393)
    at /space/rguenther/src/gcc-clean/gcc/tree-sra.cc:3961
#2  0x0000000001c07ca7 in sra_modify_call_arg (expr=0x7ffff66958c8, 
    call_gsi=0x7fffffffd730, refresh_gsi=0x7fffffffd750, flags=20)
    at /space/rguenther/src/gcc-clean/gcc/tree-sra.cc:4313

where there is no actual access (we just re-materialize one).  But AFAICS
SRA would happily analyze

  ctx = ...;
  MEM[(ref-all *)&ctx] = ...;

so which form of access to re-materialize here is not clear.  For actual
original accesses (I suppose aggregate copies) the original access base
can serve as source of info.

For the call case it would be flow-sensitive info determining the access,
but that could be even different alias-sets for stores to different parts
of the structure I think.

One way out would be to disallow accesses where the bare decl isn't
appropriate, but I fear this will pessimize things too much.  Another
compromise is to record the alias-set ptr and iff there's two different
ones pun all accesses ever materialized to alias-set zero.

It's still that build_ref_for_offset would need to have an extra argument
specifying that alias-set.

Reply via email to