Hi, 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), (call_param_p ? EXPAND_STACK_PARM : EXPAND_NORMAL), &alt_rtl); 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). Thanks, Martin 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));