For the attached Ada testcase, the compiler generates a double write to the
volatile variable at -O1. The problem is a VIEW_CONVERT_EXPR on the rhs.
We have in store_expr:
If TEMP and TARGET compare equal according to rtx_equal_p, but
one or both of them are volatile memory refs, we have to distinguish
two cases:
- expand_expr has used TARGET. In this case, we must not generate
another copy. This can be detected by TARGET being equal according
to == .
- expand_expr has not used TARGET - that means that the source just
happens to have the same RTX form. Since temp will have been created
by expand_expr, it will compare unequal according to == .
We must generate a copy in this case, to reach the correct number
of volatile memory references. */
So store_expr expects that, if expand_expr returns TEMP != TARGET, then TARGET
hasn't been used and a copy is needed.
Now in the VIEW_CONVERT_EXPR case of expand_expr, we have:
/* At this point, OP0 is in the correct mode. If the output type is
such that the operand is known to be aligned, indicate that it is.
Otherwise, we need only be concerned about alignment for non-BLKmode
results. */
if (MEM_P (op0))
{
enum insn_code icode;
op0 = copy_rtx (op0);
i.e. op0 is blindly copied, which breaks the assumption of store_expr.
The attached patch removes the copy_rtx from the main path and puts it back
only on the sub-path where it is presumably still needed. Of course it's
probably still problematic wrt store_expr, but it's the TYPE_ALIGN_OK case and
I'm not sure it matters in practice; the patch contains a ??? note though.
Tested on x86_64-suse-linux, applied on the mainline.
2012-10-15 Eric Botcazou <ebotca...@adacore.com>
* expr.c (expand_expr_real_1) <VIEW_CONVERT_EXPR>: Do not unnecessarily
copy the object in the MEM_P case.
2012-10-15 Eric Botcazou <ebotca...@adacore.com>
* gnat.dg/unchecked_convert9.ad[sb]: New test.
--
Eric Botcazou
Index: expr.c
===================================================================
--- expr.c (revision 192447)
+++ expr.c (working copy)
@@ -10270,10 +10270,15 @@ expand_expr_real_1 (tree exp, rtx target
{
enum insn_code icode;
- op0 = copy_rtx (op0);
-
if (TYPE_ALIGN_OK (type))
- set_mem_align (op0, MAX (MEM_ALIGN (op0), TYPE_ALIGN (type)));
+ {
+ /* ??? Copying the MEM without substantially changing it might
+ run afoul of the code handling volatile memory references in
+ store_expr, which assumes that TARGET is returned unmodified
+ if it has been used. */
+ op0 = copy_rtx (op0);
+ set_mem_align (op0, MAX (MEM_ALIGN (op0), TYPE_ALIGN (type)));
+ }
else if (mode != BLKmode
&& MEM_ALIGN (op0) < GET_MODE_ALIGNMENT (mode)
/* If the target does have special handling for unaligned
-- { dg-do compile }
-- { dg-options "-O -fdump-rtl-final" }
package body Unchecked_Convert9 is
procedure Proc is
L : Unsigned_32 := 16#55557777#;
begin
Var := Conv (L);
end;
end Unchecked_Convert9;
-- { dg-final { scan-rtl-dump-times "set \\(mem/v" 1 "final" } }
-- { dg-final { cleanup-rtl-dump "final" } }
with System;
with Ada.Unchecked_Conversion;
with Interfaces; use Interfaces;
package Unchecked_Convert9 is
type R is record
H : Unsigned_16;
L : Unsigned_16;
end record;
Var : R;
pragma Volatile (Var);
function Conv is new
Ada.Unchecked_Conversion (Source => Unsigned_32, Target => R);
procedure Proc;
end Unchecked_Convert9;