Patchwork [fortran] Set TREE_STATIC flag on static initializers

login
register
mail settings
Submitter Jan Hubicka
Date Sept. 4, 2010, 11:18 p.m.
Message ID <20100904231844.GF31380@kam.mff.cuni.cz>
Download mbox | patch
Permalink /patch/63812/
State New
Headers show

Comments

Jan Hubicka - Sept. 4, 2010, 11:18 p.m.
Hi,
fortran, unlike all other frontends, does not set TREE_STATIC flag on static
initializers.  This prevents us from folding till the cfgexpand time.  To be honest,
I am not quite sure why we insist on the flag (I guess we should assert that all statics
are initializerd with static initializer or ignore it), but this patch brings gfortran
into sync with others.

Bootstrapped/regtested x86_64-linux, OK?

Honza

	* trans-expr.c (gfc_conv_initializer): Set TREE_STATIC on static ctors.
Tobias Burnus - Sept. 5, 2010, 9:22 p.m.
Jan Hubicka wrote:
> fortran, unlike all other frontends, does not set TREE_STATIC flag on static
> initializers.  This prevents us from folding till the cfgexpand time.  To be honest,
> I am not quite sure why we insist on the flag (I guess we should assert that all statics
> are initializerd with static initializer or ignore it), but this patch brings gfortran
> into sync with others.
>
> Bootstrapped/regtested x86_64-linux, OK?

OK. Thanks for the patch.

Tobias

PS: It helps to CC fortran@ for Fortran patches; sending them only to 
gcc-patches@ reduces the chance of getting them reviewed.
H.J. Lu - Sept. 8, 2010, 4:21 p.m.
On Sat, Sep 4, 2010 at 4:18 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> fortran, unlike all other frontends, does not set TREE_STATIC flag on static
> initializers.  This prevents us from folding till the cfgexpand time.  To be honest,
> I am not quite sure why we insist on the flag (I guess we should assert that all statics
> are initializerd with static initializer or ignore it), but this patch brings gfortran
> into sync with others.
>
> Bootstrapped/regtested x86_64-linux, OK?
>
> Honza
>
>        * trans-expr.c (gfc_conv_initializer): Set TREE_STATIC on static ctors.

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45598
Jan Hubicka - Sept. 8, 2010, 4:28 p.m.
> On Sat, Sep 4, 2010 at 4:18 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> > Hi,
> > fortran, unlike all other frontends, does not set TREE_STATIC flag on static
> > initializers.  This prevents us from folding till the cfgexpand time.  To be honest,
> > I am not quite sure why we insist on the flag (I guess we should assert that all statics
> > are initializerd with static initializer or ignore it), but this patch brings gfortran
> > into sync with others.
> >
> > Bootstrapped/regtested x86_64-linux, OK?
> >
> > Honza
> >
> >        * trans-expr.c (gfc_conv_initializer): Set TREE_STATIC on static ctors.
> 
> This caused:
> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45598

It was previously latent.  I am testing fix for that.

Honza
H.J. Lu - Sept. 10, 2010, 1:20 p.m.
2010/9/8 Jan Hubicka <hubicka@ucw.cz>:
>> On Sat, Sep 4, 2010 at 4:18 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> > Hi,
>> > fortran, unlike all other frontends, does not set TREE_STATIC flag on static
>> > initializers.  This prevents us from folding till the cfgexpand time.  To be honest,
>> > I am not quite sure why we insist on the flag (I guess we should assert that all statics
>> > are initializerd with static initializer or ignore it), but this patch brings gfortran
>> > into sync with others.
>> >
>> > Bootstrapped/regtested x86_64-linux, OK?
>> >
>> > Honza
>> >
>> >        * trans-expr.c (gfc_conv_initializer): Set TREE_STATIC on static ctors.
>>
>> This caused:
>>
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45598
>
> It was previously latent.  I am testing fix for that.
>

Revision 164111:

http://gcc.gnu.org/ml/gcc-cvs/2010-09/msg00403.html

doesn't fix SPEC CPU 2K:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45634
H.J. Lu - Sept. 10, 2010, 2:05 p.m.
On Fri, Sep 10, 2010 at 6:20 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> 2010/9/8 Jan Hubicka <hubicka@ucw.cz>:
>>> On Sat, Sep 4, 2010 at 4:18 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>>> > Hi,
>>> > fortran, unlike all other frontends, does not set TREE_STATIC flag on static
>>> > initializers.  This prevents us from folding till the cfgexpand time.  To be honest,
>>> > I am not quite sure why we insist on the flag (I guess we should assert that all statics
>>> > are initializerd with static initializer or ignore it), but this patch brings gfortran
>>> > into sync with others.
>>> >
>>> > Bootstrapped/regtested x86_64-linux, OK?
>>> >
>>> > Honza
>>> >
>>> >        * trans-expr.c (gfc_conv_initializer): Set TREE_STATIC on static ctors.
>>>
>>> This caused:
>>>
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45598
>>
>> It was previously latent.  I am testing fix for that.
>>
>
> Revision 164111:
>
> http://gcc.gnu.org/ml/gcc-cvs/2010-09/msg00403.html

Jan, shouldn't

      /* Fold read from constant string.  */
      if (TREE_CODE (ctor) == STRING_CST)
        {
          if ((TYPE_MODE (TREE_TYPE (t))
               == TYPE_MODE (TREE_TYPE (TREE_TYPE (ctor))))
              && (GET_MODE_CLASS (TYPE_MODE (TREE_TYPE (TREE_TYPE (ctor))))
                  == MODE_INT)
              && GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (TREE_TYPE (ctor)))) == 1
              && compare_tree_int (idx, TREE_STRING_LENGTH (ctor)) < 0)
            return build_int_cst_type (TREE_TYPE (t),
                                       (TREE_STRING_POINTER (ctor)
                                        [TREE_INT_CST_LOW (idx)]));
          return NULL_TREE;
        }


in tree-ssa-ccp.c need a similar fix?
Jan Hubicka - Sept. 10, 2010, 4:41 p.m.
> On Fri, Sep 10, 2010 at 6:20 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> > 2010/9/8 Jan Hubicka <hubicka@ucw.cz>:
> >>> On Sat, Sep 4, 2010 at 4:18 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> >>> > Hi,
> >>> > fortran, unlike all other frontends, does not set TREE_STATIC flag on static
> >>> > initializers.  This prevents us from folding till the cfgexpand time.  To be honest,
> >>> > I am not quite sure why we insist on the flag (I guess we should assert that all statics
> >>> > are initializerd with static initializer or ignore it), but this patch brings gfortran
> >>> > into sync with others.
> >>> >
> >>> > Bootstrapped/regtested x86_64-linux, OK?
> >>> >
> >>> > Honza
> >>> >
> >>> >        * trans-expr.c (gfc_conv_initializer): Set TREE_STATIC on static ctors.
> >>>
> >>> This caused:
> >>>
> >>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=45598
> >>
> >> It was previously latent.  I am testing fix for that.
> >>
> >
> > Revision 164111:
> >
> > http://gcc.gnu.org/ml/gcc-cvs/2010-09/msg00403.html
> 
> Jan, shouldn't
> 
>       /* Fold read from constant string.  */
>       if (TREE_CODE (ctor) == STRING_CST)
>         {
>           if ((TYPE_MODE (TREE_TYPE (t))
>                == TYPE_MODE (TREE_TYPE (TREE_TYPE (ctor))))
>               && (GET_MODE_CLASS (TYPE_MODE (TREE_TYPE (TREE_TYPE (ctor))))
>                   == MODE_INT)
>               && GET_MODE_SIZE (TYPE_MODE (TREE_TYPE (TREE_TYPE (ctor)))) == 1
>               && compare_tree_int (idx, TREE_STRING_LENGTH (ctor)) < 0)
>             return build_int_cst_type (TREE_TYPE (t),
>                                        (TREE_STRING_POINTER (ctor)
>                                         [TREE_INT_CST_LOW (idx)]));
>           return NULL_TREE;
>         }
> 
> 
> in tree-ssa-ccp.c need a similar fix?

Yes, I am working on patch that wil unify this code, so we won't need duplication...

Honza
> 
> 
> -- 
> H.J.

Patch

Index: fortran/trans-expr.c
===================================================================
--- fortran/trans-expr.c	(revision 163862)
+++ fortran/trans-expr.c	(working copy)
@@ -3992,19 +3992,23 @@  gfc_conv_initializer (gfc_expr * expr, g
 
       gfc_init_se (&se, NULL);
       gfc_conv_constant (&se, expr);
+      gcc_assert (TREE_CODE (se.expr) != CONSTRUCTOR);
       return se.expr;
     }
   
   if (array && !procptr)
     {
+      tree ctor;
       /* Arrays need special handling.  */
       if (pointer)
-	return gfc_build_null_descriptor (type);
+	ctor = gfc_build_null_descriptor (type);
       /* Special case assigning an array to zero.  */
       else if (is_zero_initializer_p (expr))
-        return build_constructor (type, NULL);
+        ctor = build_constructor (type, NULL);
       else
-	return gfc_conv_array_initializer (type, expr);
+	ctor = gfc_conv_array_initializer (type, expr);
+      TREE_STATIC (ctor) = 1;
+      return ctor;
     }
   else if (pointer || procptr)
     {
@@ -4015,6 +4019,7 @@  gfc_conv_initializer (gfc_expr * expr, g
 	  gfc_init_se (&se, NULL);
 	  se.want_pointer = 1;
 	  gfc_conv_expr (&se, expr);
+          gcc_assert (TREE_CODE (se.expr) != CONSTRUCTOR);
 	  return se.expr;
 	}
     }
@@ -4029,14 +4034,21 @@  gfc_conv_initializer (gfc_expr * expr, g
 	    gfc_conv_structure (&se, gfc_class_null_initializer(ts), 1);
 	  else
 	    gfc_conv_structure (&se, expr, 1);
+	  gcc_assert (TREE_CODE (se.expr) == CONSTRUCTOR);
+	  TREE_STATIC (se.expr) = 1;
 	  return se.expr;
 
 	case BT_CHARACTER:
-	  return gfc_conv_string_init (ts->u.cl->backend_decl,expr);
+	  {
+	    tree ctor = gfc_conv_string_init (ts->u.cl->backend_decl,expr);
+	    TREE_STATIC (ctor) = 1;
+	    return ctor;
+	  }
 
 	default:
 	  gfc_init_se (&se, NULL);
 	  gfc_conv_constant (&se, expr);
+	  gcc_assert (TREE_CODE (se.expr) != CONSTRUCTOR);
 	  return se.expr;
 	}
     }