diff mbox

Dissociate MEM_EXPR so that it is not marked as addressable

Message ID 9293879.LMQFIBm6Yn@polaris
State New
Headers show

Commit Message

Eric Botcazou Jan. 14, 2015, 12:15 p.m. UTC 
Hi,

this is a follow-up to
  https://gcc.gnu.org/ml/gcc-patches/2012-03/msg01890.html
where Martin was fixing an Ada bootstrap failure on SPARC64.  After some 
discussion between Martin, Richard B. and me, the initial proposal evolved 
into a tweak to be applied to the existing call to set_mem_attributes in the 
normal_inner_ref case of expand_expr_real_1, first to avoid it and eventually 
to call it on the type instead of the expression.

That was an oversight on my side, since the type is that of the expression and 
not that of the value spilled onto the stack, so it may not always be seen as 
having an alias set conflicting with that of the expression in Ada, e.g. if 
the expression is a an ARRAY_REF of a VCE to an array with a non-aliased 
component, see the attached testcase.  So the patch reverts the change and 
instead clears MEM_EXPR in the problematic case, as first proposed by Martin.

Bootstrapped/regtested on x86_64-suse-linux, OK for mainline and 4.9 branch?


2015-01-14  Eric Botcazou  <ebotcazou@adacore.com>

	* expr.c (expand_expr_real_1) <normal_inner_ref>: Use the expression to
	set the memory attributes in all cases but clear MEM_EXPR if need be.


2015-01-14  Eric Botcazou  <ebotcazou@adacore.com>

	* testsuite/gnat.dg/opt47.adb: New test.

Comments

Richard Biener Jan. 14, 2015, 1:48 p.m. UTC | #1 
On Wed, Jan 14, 2015 at 1:15 PM, Eric Botcazou <ebotcazou@adacore.com> wrote:
> Hi,
>
> this is a follow-up to
>   https://gcc.gnu.org/ml/gcc-patches/2012-03/msg01890.html
> where Martin was fixing an Ada bootstrap failure on SPARC64.  After some
> discussion between Martin, Richard B. and me, the initial proposal evolved
> into a tweak to be applied to the existing call to set_mem_attributes in the
> normal_inner_ref case of expand_expr_real_1, first to avoid it and eventually
> to call it on the type instead of the expression.
>
> That was an oversight on my side, since the type is that of the expression and
> not that of the value spilled onto the stack, so it may not always be seen as
> having an alias set conflicting with that of the expression in Ada, e.g. if
> the expression is a an ARRAY_REF of a VCE to an array with a non-aliased
> component, see the attached testcase.  So the patch reverts the change and
> instead clears MEM_EXPR in the problematic case, as first proposed by Martin.
>
> Bootstrapped/regtested on x86_64-suse-linux, OK for mainline and 4.9 branch?

Ok.

Thanks,
Richard.

>
> 2015-01-14  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * expr.c (expand_expr_real_1) <normal_inner_ref>: Use the expression to
>         set the memory attributes in all cases but clear MEM_EXPR if need be.
>
>
> 2015-01-14  Eric Botcazou  <ebotcazou@adacore.com>
>
>         * testsuite/gnat.dg/opt47.adb: New test.
>
>
> --
> Eric Botcazou
diff mbox

Patch

Index: expr.c
===================================================================
--- expr.c	(revision 219578)
+++ expr.c	(working copy)
@@ -10137,7 +10137,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 mem_attrs_from_type = false;
+	bool clear_mem_expr = false;
 
 	/* If we got back the original object, something is wrong.  Perhaps
 	   we are evaluating an expression too early.  In any event, don't
@@ -10233,7 +10233,7 @@  expand_expr_real_1 (tree exp, rtx target
 	    memloc = assign_temp (TREE_TYPE (tem), 1, 1);
 	    emit_move_insn (memloc, op0);
 	    op0 = memloc;
-	    mem_attrs_from_type = true;
+	    clear_mem_expr = true;
 	  }
 
 	if (offset)
@@ -10417,17 +10417,17 @@  expand_expr_real_1 (tree exp, rtx target
 	if (op0 == orig_op0)
 	  op0 = copy_rtx (op0);
 
-	/* If op0 is a temporary because of forcing to memory, pass only the
-	   type to set_mem_attributes so that the original expression is never
-	   marked as ADDRESSABLE through MEM_EXPR of the temporary.  */
-	if (mem_attrs_from_type)
-	  set_mem_attributes (op0, type, 0);
-	else
-	  set_mem_attributes (op0, exp, 0);
+	set_mem_attributes (op0, exp, 0);
 
 	if (REG_P (XEXP (op0, 0)))
 	  mark_reg_pointer (XEXP (op0, 0), MEM_ALIGN (op0));
 
+	/* If op0 is a temporary because the original expressions was forced
+	   to memory, clear MEM_EXPR so that the original expression cannot
+	   be marked as addressable through MEM_EXPR of the temporary.  */
+	if (clear_mem_expr)
+	  set_mem_expr (op0, NULL_TREE);
+
 	MEM_VOLATILE_P (op0) |= volatilep;
 	if (mode == mode1 || mode1 == BLKmode || mode1 == tmode
 	    || modifier == EXPAND_CONST_ADDRESS