On Wed, Apr 25, 2012 at 3:37 PM, Olivier Hainque <hain...@adacore.com> wrote:
> Hello,
>
> For the "PA(1).Z := 44;" assignment in the attached Ada
> testcase, we observe the gcc 4.5 SRA pass performing an
> invalid transformation, turning:
>
> struct {
> system__pack_48__bits_48 OBJ;
> } D.1432;
>
> D.1432.OBJ = D.1435;
> T1b.F = VIEW_CONVERT_EXPR<struct pt__point>(D.1432);
>
> into:
>
> SR.12_17 = D.1435_3;
> T1b.F = VIEW_CONVERT_EXPR<struct pt__point>(SR.12_17);
>
> where we have
>
> <var_decl D.1432
> type <record_type 0x7ffff7fb72a0 BLK
> size <integer_cst 0x7ffff7fac960 constant 48>
>
> and
>
> <var_decl SR.12
> type <integer_type system__pack_48__bits_48
> size <integer_cst 0x7ffff7ecd870 64>
>
> type <integer_type system__pack_48__bits_48___UMT
> size <integer_cst 64>
>
> At least the change in size is problematic because the conversion
> outcome might differ after the replacement, in particular on
> big-endian targets.
>
> mainline does something slightly different with the same effects
> eventually (same testcase failure on sparc-solaris). The attached patch
> is a proposal to address this at the point where we already check
> for VCE in the access creation process, disqualifying scalarization
> for a VCE argument of non-integral size.
>
> We (AdaCore) have been using this internally for a while now.
> I also checked that it fixes the observable problem on sparc, then
> bootstrapped and regtested on i686-suse-linux.
>
> OK to commit ?
>
> Thanks in advance for your feedback,
Does this really cover all problematic cases? Ah, the existing code
already rejects all VIEW_CONVERT_EXPRs down in the reference
chain.
I think much better would be to simply disallow any toplevel
VIEW_CONVERT_EXPR of BLKmode, so, something like
Index: gcc/tree-sra.c
===================================================================
--- gcc/tree-sra.c (revision 186817)
+++ gcc/tree-sra.c (working copy)
@@ -1001,9 +1001,10 @@ build_access_from_expr_1 (tree expr, gim
/* We need to dive through V_C_Es in order to get the size of its parameter
and not the result type. Ada produces such statements. We are also
- capable of handling the topmost V_C_E but not any of those buried in other
- handled components. */
- if (TREE_CODE (expr) == VIEW_CONVERT_EXPR)
+ capable of handling the topmost V_C_E if it has a suitable mode but
+ not any of those buried in other handled components. */
+ if (TREE_CODE (expr) == VIEW_CONVERT_EXPR
+ && TYPE_MODE (TREE_TYPE (expr)) != BLKmode)
expr = TREE_OPERAND (expr, 0);
if (contains_view_convert_expr_p (expr))
Does that fix your problems, too? If so I prefer that.
Thanks,
Richard.
> Olivier
>
> 2012-04-25 Olivier Hainque <hain...@adacore.com>
>
> * tree-sra.c (create_access): Additional IN_VCE argument, telling
> if EXPR is VIEW_CONVERT_EXPR input. Disqualify base if access size
> is not that of an integer mode in this case.
> (build_access_from_expr_1): Adjust caller.
>
> testsuite/
> * gnat.dg/sra_vce[_decls].adb: New testcase.
>
