Fix problems with tree-sra.c with VIEW_CONVERT_EXPR

[Diego Novillo]

On Mon, 2004-06-28 at 15:01, Richard Kenner wrote:
>     > That returns NULL_TREE is it's not an SSA_VAR_P.
> 
>     Huh?  It does exactly what you added to is_sra_candidate_complex_ref.
> 
> The first part is the same, but then get_base_address checks that what
> it has is an SSA_VAR_P a STRING_CST, a CONSTRUCTOR or an INDIRECT_REF
> and returns NULL_TREE if it isn't one of those.  But here we're
> looking for a VAR_DECL and one that's been scalarized.
> 
/* Nonzero if DECL represents a variable for the SSA passes.  */
#define SSA_VAR_P(DECL) \
        (TREE_CODE (DECL) == VAR_DECL   \
         || TREE_CODE (DECL) == PARM_DECL \
         || TREE_CODE (DECL) == RESULT_DECL \
         [ ... ]


> Can you show me the code you are suggesting be there instead of what
> I wrote?
> 
exp = get_base_address (exp);

Also, the new predicate is_sra_candidate_complex_ref returns 2 different
things.  I don't find that appealing.


>     Are you trying to handle some kind of structure nesting?  Can you give
>     me an example?
> 
> I did, in the original patch file, but the case I saw was
>     <MODIFY_EXPR v1 <VIEW_CONVERT_EXPR v2>>
> where both v1 and v2 had been scalarized.
>
In that case I am completely lost.  I don't understand why you added all
that code.  Particularly, in here:

---------------------------------------------------------------------------
*************** scalarize_modify_expr (block_stmt_iterat
*************** scalarize_modify_expr (block_stmt_iterat
       preventing the optimizers from removing all the redundant
       operations.  */
!   else if (is_sra_candidate_ref (rhs, true))
      {
-       tree var = TREE_OPERAND (rhs, 0);
        emit_scalar_copies (si_p, var, var, FIELD_SCALAR);
      }
  
--- 969,988 ----
       preventing the optimizers from removing all the redundant
       operations.  */
!   else if (is_sra_candidate_complex_ref (rhs, &var))
      {
        emit_scalar_copies (si_p, var, var, FIELD_SCALAR);
+ 
+       /* If the LHS of the assignment is also a scalarizable
structure, insert
+        copies into the scalar replacements after the call.  */
+       if (is_sra_candidate_decl (lhs))
+       {
+         tree list = create_scalar_copies (lhs, lhs, SCALAR_FIELD);
+         if (EXPR_HAS_LOCATION (stmt))
+           annotate_all_with_locus (&list, EXPR_LOCATION (stmt));
+         if (stmt_ends_bb_p (stmt))
+           insert_edge_copies (list, bb_for_stmt (stmt));
+         else
+           bsi_insert_after (si_p, list, BSI_NEW_STMT);
+       }
      }
---------------------------------------------------------------------------

After we have called 'emit_scalar_copies', you are calling
create_scalar_copies again.  Why?  At that point we have already
scalarized the assignment.

I don't think your patch is correct then.  Why not the obvious minimal
change?  All you seem to be looking for here is to handle
V1 = VIEW_CONVERT_EXPR <V2> the same way we handle V1 = V2.  The patch
below will do that and the scalarization will be handled by the DECL =
DECL case in scalarize_modify_expr.


Index: tree-sra.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-sra.c,v
retrieving revision 2.8
diff -d -u -p -r2.8 tree-sra.c
--- tree-sra.c  22 Jun 2004 03:06:41 -0000      2.8
+++ tree-sra.c  28 Jun 2004 19:58:01 -0000
@@ -917,6 +917,9 @@ scalarize_modify_expr (block_stmt_iterat
   tree lhs = TREE_OPERAND (stmt, 0);
   tree rhs = TREE_OPERAND (stmt, 1);
  
+  if (TREE_CODE (rhs) == VIEW_CONVERT_EXPR)
+    rhs = TREE_OPERAND (rhs, 0);
+
   /* Found AGGREGATE.FIELD = ...  */
   if (is_sra_candidate_ref (lhs, false))
     {



Diego.