On Fri, Mar 30, 2012 at 10:03:59AM +0200, Richard Guenther wrote:
> On Fri, 30 Mar 2012, Martin Jambor wrote:
> > Hi,
> > 
> > when testing a patch of mine on sparc64-linux, I came across an Ada
> > bootstrap failure due to a structure DECL which was marked addressable
> > but had a register DECL_RTL (and therefore mem_ref_refers_to_non_mem_p
> > failed to trigger on it).
> > 
> > Mode of the structure was TI (16 bytes int) and it was mistakenly
> > marked as addressable during expansion of an assignment statement in
> > which a 12 byte portion of it was copied to another structure with
> > BLKmode.  Specifically, this happened because expand_assignment called
> > store_expr which loaded the required portion to temporary 12 byte
> > BLKmode MEM_P variable and then called emit_block_move from the
> > temporary to the destination.  emit_block_move_hints then marked
> > MEM_EXPR of the temp as addressable because it handled the copy by
> > emitting a library call.  And MEM_EXPR pointed to the DECL of the
> > source of the assignment which I believe is the bug, thus this patch
> > re-sets MEM_EXPR of temp in these cases.
> > 
> > However, if anybody believes the main issue is elsewhere and another
> > component of this chain of events needs to be fixed, I'll be happy to
> > come up with another patch.  so far this patch has passed bootstrap
> > and testing on x86_64-linux and helped my patch which uncovered this
> > issue to reach stage 3 of bootstrap.
> > 
> > What do you think, is it OK for trunk?
> Hmm, I think this should be done at the point we create the mem - where
> does that happen?

The function gets it by simply calling expand_expr_real:

      temp = expand_expr_real (exp, tmp_target, GET_MODE (target),
                                ? EXPAND_STACK_PARM : EXPAND_NORMAL),

I have reproduced the issue again and looked at how expand_expr_real_1
comes up with the MEM attributes in the COMPONENT_REF case.
The memory-backed temporary is created in:

        /* Otherwise, if this is a constant or the object is not in memory
           and need be, put it there.  */
        else if (CONSTANT_P (op0) || (!MEM_P (op0) && must_force_mem))
            tree nt = build_qualified_type (TREE_TYPE (tem),
                                            (TYPE_QUALS (TREE_TYPE (tem))
                                             | TYPE_QUAL_CONST));
            memloc = assign_temp (nt, 1, 1, 1);
            emit_move_insn (memloc, op0);
            op0 = memloc;

and at the end of the  case, the bitpos is added by adjust_address,
followed by set_mem_attributes (op0, exp, 0);

Indeed it seems that we should not be calling set_mem_attributes if
op0 is based on such temporary... or perhaps just make sure that we
only clear MEM_EXPR afterwards?

For example, the following patch also passes bootstrap and testing on
x86_64-linux, bootstrap on sparc64 is still running (and IMHO has not
yet reached the critical point).



Index: gcc/expr.c
--- gcc/expr.c  (revision 185792)
+++ gcc/expr.c  (working copy)
@@ -9564,6 +9564,7 @@ expand_expr_real_1 (tree exp, rtx target
        tree tem = get_inner_reference (exp, &bitsize, &bitpos, &offset,
                                        &mode1, &unsignedp, &volatilep, true);
        rtx orig_op0, memloc;
+       bool do_set_mem_attrs = true;
        /* If we got back the original object, something is wrong.  Perhaps
           we are evaluating an expression too early.  In any event, don't
@@ -9669,6 +9670,7 @@ expand_expr_real_1 (tree exp, rtx target
            memloc = assign_temp (nt, 1, 1, 1);
            emit_move_insn (memloc, op0);
            op0 = memloc;
+           do_set_mem_attrs = false;
        if (offset)
@@ -9841,7 +9843,8 @@ expand_expr_real_1 (tree exp, rtx target
                emit_move_insn (new_rtx, op0);
                op0 = copy_rtx (new_rtx);
                PUT_MODE (op0, BLKmode);
-               set_mem_attributes (op0, exp, 1);
+               if (do_set_mem_attrs)
+                 set_mem_attributes (op0, exp, 1);
            return op0;
@@ -9862,7 +9865,8 @@ expand_expr_real_1 (tree exp, rtx target
        if (op0 == orig_op0)
          op0 = copy_rtx (op0);
-       set_mem_attributes (op0, exp, 0);
+       if (do_set_mem_attrs)
+         set_mem_attributes (op0, exp, 0);
        if (REG_P (XEXP (op0, 0)))
          mark_reg_pointer (XEXP (op0, 0), MEM_ALIGN (op0));

Reply via email to