Fix PR50260

Fix PR50260

[Michael Matz]

Hi,

the last change in ipa-split generated a new use of a previously unused 
PARM_DECL.  When one does this one has to call add_referenced_var.  Not 
doing so can cause segfault when accessing the (not initialized) var 
annotation.  So, fixed with the patch.

I took the opportunity to remove all explicit calls to get_var_ann because 
add_referenced_var is doing so as first thing.  Reviewing the code showed 
one potentially problematic case in tree-ssa-pre.c where a new variable 
was created without calling add_referenced_var.  It's only for 
place-holder SSA names but those can conceivably leak into the program 
stream by being reused during expression generation.

Currently regstrapping on x86_64-linux (without Ada).  Okay for trunk?


Ciao,
Michael.
-- 
	PR middle-end/50260
	* ipa-split.c (split_function): Call add_referenced_var.

	* tree-ssa-phiopt.c (cond_store_replacement): Don't call get_var_ann.
	(cond_if_else_store_replacement_1): Ditto.
	* tree-ssa-pre.c (get_representative_for): Ditto.
	(create_expression_by_pieces): Ditto.
	(insert_into_preds_of_block): Ditto.
	* tree-sra.c (create_access_replacement): Ditto.
	(get_replaced_param_substitute): Ditto.

testsuite/
	* gfortran.fortran-torture/compile/pr50260.f90: New test.

Index: ipa-split.c
===================================================================
--- ipa-split.c	(revision 178408)
+++ ipa-split.c	(working copy)
@@ -988,6 +988,9 @@ split_function (struct split_point *spli
 	arg = gimple_default_def (cfun, parm);
 	if (!arg)
 	  {
+	    /* This parm wasn't used up to now, but is going to be used,
+	       hence register it.  */
+	    add_referenced_var (parm);
 	    arg = make_ssa_name (parm, gimple_build_nop ());
 	    set_default_def (parm, arg);
 	  }
Index: tree-ssa-phiopt.c
===================================================================
--- tree-ssa-phiopt.c	(revision 178408)
+++ tree-ssa-phiopt.c	(working copy)
@@ -1269,10 +1269,7 @@ cond_store_replacement (basic_block midd
   /* 2) Create a temporary where we can store the old content
         of the memory touched by the store, if we need to.  */
   if (!condstoretemp || TREE_TYPE (lhs) != TREE_TYPE (condstoretemp))
-    {
-      condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
-      get_var_ann (condstoretemp);
-    }
+    condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
   add_referenced_var (condstoretemp);
 
   /* 3) Insert a load from the memory of the store to the temporary
@@ -1355,10 +1352,7 @@ cond_if_else_store_replacement_1 (basic_
   /* 2) Create a temporary where we can store the old content
 	of the memory touched by the store, if we need to.  */
   if (!condstoretemp || TREE_TYPE (lhs) != TREE_TYPE (condstoretemp))
-    {
-      condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
-      get_var_ann (condstoretemp);
-    }
+    condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
   add_referenced_var (condstoretemp);
 
   /* 3) Create a PHI node at the join block, with one argument
Index: tree-ssa-pre.c
===================================================================
--- tree-ssa-pre.c	(revision 178408)
+++ tree-ssa-pre.c	(working copy)
@@ -1399,7 +1399,7 @@ get_representative_for (const pre_expr e
   if (!pretemp || exprtype != TREE_TYPE (pretemp))
     {
       pretemp = create_tmp_reg (exprtype, "pretmp");
-      get_var_ann (pretemp);
+      add_referenced_var (pretemp);
     }
 
   name = make_ssa_name (pretemp, gimple_build_nop ());
@@ -3178,10 +3178,7 @@ create_expression_by_pieces (basic_block
   /* Build and insert the assignment of the end result to the temporary
      that we will return.  */
   if (!pretemp || exprtype != TREE_TYPE (pretemp))
-    {
-      pretemp = create_tmp_reg (exprtype, "pretmp");
-      get_var_ann (pretemp);
-    }
+    pretemp = create_tmp_reg (exprtype, "pretmp");
 
   temp = pretemp;
   add_referenced_var (temp);
@@ -3441,10 +3438,7 @@ insert_into_preds_of_block (basic_block
 
   /* Now build a phi for the new variable.  */
   if (!prephitemp || TREE_TYPE (prephitemp) != type)
-    {
-      prephitemp = create_tmp_var (type, "prephitmp");
-      get_var_ann (prephitemp);
-    }
+    prephitemp = create_tmp_var (type, "prephitmp");
 
   temp = prephitemp;
   add_referenced_var (temp);
Index: tree-sra.c
===================================================================
--- tree-sra.c	(revision 178408)
+++ tree-sra.c	(working copy)
@@ -1825,7 +1825,6 @@ create_access_replacement (struct access
   tree repl;
 
   repl = create_tmp_var (access->type, "SR");
-  get_var_ann (repl);
   add_referenced_var (repl);
   if (rename)
     mark_sym_for_renaming (repl);
@@ -4106,7 +4105,6 @@ get_replaced_param_substitute (struct ip
       DECL_NAME (repl) = get_identifier (pretty_name);
       obstack_free (&name_obstack, pretty_name);
 
-      get_var_ann (repl);
       add_referenced_var (repl);
       adj->new_ssa_base = repl;
     }
Index: testsuite/gfortran.fortran-torture/compile/pr50260.f90
===================================================================
--- testsuite/gfortran.fortran-torture/compile/pr50260.f90	(revision 0)
+++ testsuite/gfortran.fortran-torture/compile/pr50260.f90	(revision 0)
@@ -0,0 +1,48 @@
+MODULE cp_parser_methods
+  INTEGER, PARAMETER :: default_string_length=80
+  INTEGER, PARAMETER :: default_path_length=250
+  TYPE ilist_type
+     LOGICAL                              :: in_use
+  END TYPE ilist_type
+  TYPE cp_parser_type
+     CHARACTER(LEN=default_path_length)             :: ifn
+     INTEGER                                        :: icol,icol1,icol2
+     TYPE(ilist_type), POINTER                      :: ilist
+  END TYPE cp_parser_type
+  TYPE cp_error_type
+  END TYPE cp_error_type
+CONTAINS
+  FUNCTION cts(i) RESULT(res)
+    CHARACTER(len=6)                         :: res
+  END FUNCTION cts
+  FUNCTION parser_location(parser,error) RESULT(res)
+    TYPE(cp_parser_type), POINTER            :: parser
+    TYPE(cp_error_type), INTENT(inout)       :: error
+    CHARACTER(len=default_path_length+default_string_length)       :: res
+    LOGICAL                                  :: failure
+    IF (.NOT. failure) THEN
+       res="file:'"//TRIM(parser%ifn)//"' line:"//cts(parser%icol)
+    END IF
+  END FUNCTION parser_location
+  SUBROUTINE parser_get_integer(parser,at_end, error)
+    TYPE(cp_parser_type), POINTER            :: parser
+    TYPE(cp_error_type), INTENT(inout)       :: error
+    LOGICAL                                  :: failure, my_at_end
+    IF (.NOT.failure) THEN
+       IF (.NOT.parser%ilist%in_use) THEN
+          CALL cp_assert("A"// TRIM(parser_location(parser,error)))
+       END IF
+    END IF
+  END SUBROUTINE parser_get_integer
+  SUBROUTINE parser_get_string(parser,at_end,error)
+    TYPE(cp_parser_type), POINTER            :: parser
+    LOGICAL, INTENT(out), OPTIONAL           :: at_end
+    TYPE(cp_error_type), INTENT(inout)       :: error
+    LOGICAL                                  :: failure, my_at_end
+    IF (.NOT.failure) THEN
+       IF (PRESENT(at_end)) THEN
+          CALL cp_assert("s"//TRIM(parser_location(parser,error)))
+       END IF
+    END IF
+  END SUBROUTINE parser_get_string
+END MODULE cp_parser_methods

Re: Fix PR50260

[Martin Jambor]

Hi,

On Thu, Sep 01, 2011 at 02:41:15PM +0200, Michael Matz wrote:
> Hi,
> 
> the last change in ipa-split generated a new use of a previously unused 
> PARM_DECL.  When one does this one has to call add_referenced_var.  Not 
> doing so can cause segfault when accessing the (not initialized) var 
> annotation.  So, fixed with the patch.

This is how I have fixed the issue when I ran into it independently
about an hour ago and it is probably my preferred way of dealing with
this, at least for now (that is until Honza makes up his mind what to
do with the type attributes in fnsplit).

Thanks a lot,

Martin

> 
> I took the opportunity to remove all explicit calls to get_var_ann because 
> add_referenced_var is doing so as first thing.  Reviewing the code showed 
> one potentially problematic case in tree-ssa-pre.c where a new variable 
> was created without calling add_referenced_var.  It's only for 
> place-holder SSA names but those can conceivably leak into the program 
> stream by being reused during expression generation.
> 
> Currently regstrapping on x86_64-linux (without Ada).  Okay for trunk?
> 
> 
> Ciao,
> Michael.
> -- 
> 	PR middle-end/50260
> 	* ipa-split.c (split_function): Call add_referenced_var.
> 
> 	* tree-ssa-phiopt.c (cond_store_replacement): Don't call get_var_ann.
> 	(cond_if_else_store_replacement_1): Ditto.
> 	* tree-ssa-pre.c (get_representative_for): Ditto.
> 	(create_expression_by_pieces): Ditto.
> 	(insert_into_preds_of_block): Ditto.
> 	* tree-sra.c (create_access_replacement): Ditto.
> 	(get_replaced_param_substitute): Ditto.
> 
> testsuite/
> 	* gfortran.fortran-torture/compile/pr50260.f90: New test.
> 
> Index: ipa-split.c
> ===================================================================
> --- ipa-split.c	(revision 178408)
> +++ ipa-split.c	(working copy)
> @@ -988,6 +988,9 @@ split_function (struct split_point *spli
>  	arg = gimple_default_def (cfun, parm);
>  	if (!arg)
>  	  {
> +	    /* This parm wasn't used up to now, but is going to be used,
> +	       hence register it.  */
> +	    add_referenced_var (parm);
>  	    arg = make_ssa_name (parm, gimple_build_nop ());
>  	    set_default_def (parm, arg);
>  	  }
> Index: tree-ssa-phiopt.c
> ===================================================================
> --- tree-ssa-phiopt.c	(revision 178408)
> +++ tree-ssa-phiopt.c	(working copy)
> @@ -1269,10 +1269,7 @@ cond_store_replacement (basic_block midd
>    /* 2) Create a temporary where we can store the old content
>          of the memory touched by the store, if we need to.  */
>    if (!condstoretemp || TREE_TYPE (lhs) != TREE_TYPE (condstoretemp))
> -    {
> -      condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
> -      get_var_ann (condstoretemp);
> -    }
> +    condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
>    add_referenced_var (condstoretemp);
>  
>    /* 3) Insert a load from the memory of the store to the temporary
> @@ -1355,10 +1352,7 @@ cond_if_else_store_replacement_1 (basic_
>    /* 2) Create a temporary where we can store the old content
>  	of the memory touched by the store, if we need to.  */
>    if (!condstoretemp || TREE_TYPE (lhs) != TREE_TYPE (condstoretemp))
> -    {
> -      condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
> -      get_var_ann (condstoretemp);
> -    }
> +    condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
>    add_referenced_var (condstoretemp);
>  
>    /* 3) Create a PHI node at the join block, with one argument
> Index: tree-ssa-pre.c
> ===================================================================
> --- tree-ssa-pre.c	(revision 178408)
> +++ tree-ssa-pre.c	(working copy)
> @@ -1399,7 +1399,7 @@ get_representative_for (const pre_expr e
>    if (!pretemp || exprtype != TREE_TYPE (pretemp))
>      {
>        pretemp = create_tmp_reg (exprtype, "pretmp");
> -      get_var_ann (pretemp);
> +      add_referenced_var (pretemp);
>      }
>  
>    name = make_ssa_name (pretemp, gimple_build_nop ());
> @@ -3178,10 +3178,7 @@ create_expression_by_pieces (basic_block
>    /* Build and insert the assignment of the end result to the temporary
>       that we will return.  */
>    if (!pretemp || exprtype != TREE_TYPE (pretemp))
> -    {
> -      pretemp = create_tmp_reg (exprtype, "pretmp");
> -      get_var_ann (pretemp);
> -    }
> +    pretemp = create_tmp_reg (exprtype, "pretmp");
>  
>    temp = pretemp;
>    add_referenced_var (temp);
> @@ -3441,10 +3438,7 @@ insert_into_preds_of_block (basic_block
>  
>    /* Now build a phi for the new variable.  */
>    if (!prephitemp || TREE_TYPE (prephitemp) != type)
> -    {
> -      prephitemp = create_tmp_var (type, "prephitmp");
> -      get_var_ann (prephitemp);
> -    }
> +    prephitemp = create_tmp_var (type, "prephitmp");
>  
>    temp = prephitemp;
>    add_referenced_var (temp);
> Index: tree-sra.c
> ===================================================================
> --- tree-sra.c	(revision 178408)
> +++ tree-sra.c	(working copy)
> @@ -1825,7 +1825,6 @@ create_access_replacement (struct access
>    tree repl;
>  
>    repl = create_tmp_var (access->type, "SR");
> -  get_var_ann (repl);
>    add_referenced_var (repl);
>    if (rename)
>      mark_sym_for_renaming (repl);
> @@ -4106,7 +4105,6 @@ get_replaced_param_substitute (struct ip
>        DECL_NAME (repl) = get_identifier (pretty_name);
>        obstack_free (&name_obstack, pretty_name);
>  
> -      get_var_ann (repl);
>        add_referenced_var (repl);
>        adj->new_ssa_base = repl;
>      }
> Index: testsuite/gfortran.fortran-torture/compile/pr50260.f90
> ===================================================================
> --- testsuite/gfortran.fortran-torture/compile/pr50260.f90	(revision 0)
> +++ testsuite/gfortran.fortran-torture/compile/pr50260.f90	(revision 0)
> @@ -0,0 +1,48 @@
> +MODULE cp_parser_methods
> +  INTEGER, PARAMETER :: default_string_length=80
> +  INTEGER, PARAMETER :: default_path_length=250
> +  TYPE ilist_type
> +     LOGICAL                              :: in_use
> +  END TYPE ilist_type
> +  TYPE cp_parser_type
> +     CHARACTER(LEN=default_path_length)             :: ifn
> +     INTEGER                                        :: icol,icol1,icol2
> +     TYPE(ilist_type), POINTER                      :: ilist
> +  END TYPE cp_parser_type
> +  TYPE cp_error_type
> +  END TYPE cp_error_type
> +CONTAINS
> +  FUNCTION cts(i) RESULT(res)
> +    CHARACTER(len=6)                         :: res
> +  END FUNCTION cts
> +  FUNCTION parser_location(parser,error) RESULT(res)
> +    TYPE(cp_parser_type), POINTER            :: parser
> +    TYPE(cp_error_type), INTENT(inout)       :: error
> +    CHARACTER(len=default_path_length+default_string_length)       :: res
> +    LOGICAL                                  :: failure
> +    IF (.NOT. failure) THEN
> +       res="file:'"//TRIM(parser%ifn)//"' line:"//cts(parser%icol)
> +    END IF
> +  END FUNCTION parser_location
> +  SUBROUTINE parser_get_integer(parser,at_end, error)
> +    TYPE(cp_parser_type), POINTER            :: parser
> +    TYPE(cp_error_type), INTENT(inout)       :: error
> +    LOGICAL                                  :: failure, my_at_end
> +    IF (.NOT.failure) THEN
> +       IF (.NOT.parser%ilist%in_use) THEN
> +          CALL cp_assert("A"// TRIM(parser_location(parser,error)))
> +       END IF
> +    END IF
> +  END SUBROUTINE parser_get_integer
> +  SUBROUTINE parser_get_string(parser,at_end,error)
> +    TYPE(cp_parser_type), POINTER            :: parser
> +    LOGICAL, INTENT(out), OPTIONAL           :: at_end
> +    TYPE(cp_error_type), INTENT(inout)       :: error
> +    LOGICAL                                  :: failure, my_at_end
> +    IF (.NOT.failure) THEN
> +       IF (PRESENT(at_end)) THEN
> +          CALL cp_assert("s"//TRIM(parser_location(parser,error)))
> +       END IF
> +    END IF
> +  END SUBROUTINE parser_get_string
> +END MODULE cp_parser_methods

Re: Fix PR50260

[Richard Guenther]

On Thu, Sep 1, 2011 at 2:41 PM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> the last change in ipa-split generated a new use of a previously unused
> PARM_DECL.  When one does this one has to call add_referenced_var.  Not
> doing so can cause segfault when accessing the (not initialized) var
> annotation.  So, fixed with the patch.
>
> I took the opportunity to remove all explicit calls to get_var_ann because
> add_referenced_var is doing so as first thing.  Reviewing the code showed
> one potentially problematic case in tree-ssa-pre.c where a new variable
> was created without calling add_referenced_var.  It's only for
> place-holder SSA names but those can conceivably leak into the program
> stream by being reused during expression generation.
>
> Currently regstrapping on x86_64-linux (without Ada).  Okay for trunk?

Ok.  Time to make get_var_ann private?

Richard.

>
> Ciao,
> Michael.
> --
>        PR middle-end/50260
>        * ipa-split.c (split_function): Call add_referenced_var.
>
>        * tree-ssa-phiopt.c (cond_store_replacement): Don't call get_var_ann.
>        (cond_if_else_store_replacement_1): Ditto.
>        * tree-ssa-pre.c (get_representative_for): Ditto.
>        (create_expression_by_pieces): Ditto.
>        (insert_into_preds_of_block): Ditto.
>        * tree-sra.c (create_access_replacement): Ditto.
>        (get_replaced_param_substitute): Ditto.
>
> testsuite/
>        * gfortran.fortran-torture/compile/pr50260.f90: New test.
>
> Index: ipa-split.c
> ===================================================================
> --- ipa-split.c (revision 178408)
> +++ ipa-split.c (working copy)
> @@ -988,6 +988,9 @@ split_function (struct split_point *spli
>        arg = gimple_default_def (cfun, parm);
>        if (!arg)
>          {
> +           /* This parm wasn't used up to now, but is going to be used,
> +              hence register it.  */
> +           add_referenced_var (parm);
>            arg = make_ssa_name (parm, gimple_build_nop ());
>            set_default_def (parm, arg);
>          }
> Index: tree-ssa-phiopt.c
> ===================================================================
> --- tree-ssa-phiopt.c   (revision 178408)
> +++ tree-ssa-phiopt.c   (working copy)
> @@ -1269,10 +1269,7 @@ cond_store_replacement (basic_block midd
>   /* 2) Create a temporary where we can store the old content
>         of the memory touched by the store, if we need to.  */
>   if (!condstoretemp || TREE_TYPE (lhs) != TREE_TYPE (condstoretemp))
> -    {
> -      condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
> -      get_var_ann (condstoretemp);
> -    }
> +    condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
>   add_referenced_var (condstoretemp);
>
>   /* 3) Insert a load from the memory of the store to the temporary
> @@ -1355,10 +1352,7 @@ cond_if_else_store_replacement_1 (basic_
>   /* 2) Create a temporary where we can store the old content
>        of the memory touched by the store, if we need to.  */
>   if (!condstoretemp || TREE_TYPE (lhs) != TREE_TYPE (condstoretemp))
> -    {
> -      condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
> -      get_var_ann (condstoretemp);
> -    }
> +    condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
>   add_referenced_var (condstoretemp);
>
>   /* 3) Create a PHI node at the join block, with one argument
> Index: tree-ssa-pre.c
> ===================================================================
> --- tree-ssa-pre.c      (revision 178408)
> +++ tree-ssa-pre.c      (working copy)
> @@ -1399,7 +1399,7 @@ get_representative_for (const pre_expr e
>   if (!pretemp || exprtype != TREE_TYPE (pretemp))
>     {
>       pretemp = create_tmp_reg (exprtype, "pretmp");
> -      get_var_ann (pretemp);
> +      add_referenced_var (pretemp);
>     }
>
>   name = make_ssa_name (pretemp, gimple_build_nop ());
> @@ -3178,10 +3178,7 @@ create_expression_by_pieces (basic_block
>   /* Build and insert the assignment of the end result to the temporary
>      that we will return.  */
>   if (!pretemp || exprtype != TREE_TYPE (pretemp))
> -    {
> -      pretemp = create_tmp_reg (exprtype, "pretmp");
> -      get_var_ann (pretemp);
> -    }
> +    pretemp = create_tmp_reg (exprtype, "pretmp");
>
>   temp = pretemp;
>   add_referenced_var (temp);
> @@ -3441,10 +3438,7 @@ insert_into_preds_of_block (basic_block
>
>   /* Now build a phi for the new variable.  */
>   if (!prephitemp || TREE_TYPE (prephitemp) != type)
> -    {
> -      prephitemp = create_tmp_var (type, "prephitmp");
> -      get_var_ann (prephitemp);
> -    }
> +    prephitemp = create_tmp_var (type, "prephitmp");
>
>   temp = prephitemp;
>   add_referenced_var (temp);
> Index: tree-sra.c
> ===================================================================
> --- tree-sra.c  (revision 178408)
> +++ tree-sra.c  (working copy)
> @@ -1825,7 +1825,6 @@ create_access_replacement (struct access
>   tree repl;
>
>   repl = create_tmp_var (access->type, "SR");
> -  get_var_ann (repl);
>   add_referenced_var (repl);
>   if (rename)
>     mark_sym_for_renaming (repl);
> @@ -4106,7 +4105,6 @@ get_replaced_param_substitute (struct ip
>       DECL_NAME (repl) = get_identifier (pretty_name);
>       obstack_free (&name_obstack, pretty_name);
>
> -      get_var_ann (repl);
>       add_referenced_var (repl);
>       adj->new_ssa_base = repl;
>     }
> Index: testsuite/gfortran.fortran-torture/compile/pr50260.f90
> ===================================================================
> --- testsuite/gfortran.fortran-torture/compile/pr50260.f90      (revision 0)
> +++ testsuite/gfortran.fortran-torture/compile/pr50260.f90      (revision 0)
> @@ -0,0 +1,48 @@
> +MODULE cp_parser_methods
> +  INTEGER, PARAMETER :: default_string_length=80
> +  INTEGER, PARAMETER :: default_path_length=250
> +  TYPE ilist_type
> +     LOGICAL                              :: in_use
> +  END TYPE ilist_type
> +  TYPE cp_parser_type
> +     CHARACTER(LEN=default_path_length)             :: ifn
> +     INTEGER                                        :: icol,icol1,icol2
> +     TYPE(ilist_type), POINTER                      :: ilist
> +  END TYPE cp_parser_type
> +  TYPE cp_error_type
> +  END TYPE cp_error_type
> +CONTAINS
> +  FUNCTION cts(i) RESULT(res)
> +    CHARACTER(len=6)                         :: res
> +  END FUNCTION cts
> +  FUNCTION parser_location(parser,error) RESULT(res)
> +    TYPE(cp_parser_type), POINTER            :: parser
> +    TYPE(cp_error_type), INTENT(inout)       :: error
> +    CHARACTER(len=default_path_length+default_string_length)       :: res
> +    LOGICAL                                  :: failure
> +    IF (.NOT. failure) THEN
> +       res="file:'"//TRIM(parser%ifn)//"' line:"//cts(parser%icol)
> +    END IF
> +  END FUNCTION parser_location
> +  SUBROUTINE parser_get_integer(parser,at_end, error)
> +    TYPE(cp_parser_type), POINTER            :: parser
> +    TYPE(cp_error_type), INTENT(inout)       :: error
> +    LOGICAL                                  :: failure, my_at_end
> +    IF (.NOT.failure) THEN
> +       IF (.NOT.parser%ilist%in_use) THEN
> +          CALL cp_assert("A"// TRIM(parser_location(parser,error)))
> +       END IF
> +    END IF
> +  END SUBROUTINE parser_get_integer
> +  SUBROUTINE parser_get_string(parser,at_end,error)
> +    TYPE(cp_parser_type), POINTER            :: parser
> +    LOGICAL, INTENT(out), OPTIONAL           :: at_end
> +    TYPE(cp_error_type), INTENT(inout)       :: error
> +    LOGICAL                                  :: failure, my_at_end
> +    IF (.NOT.failure) THEN
> +       IF (PRESENT(at_end)) THEN
> +          CALL cp_assert("s"//TRIM(parser_location(parser,error)))
> +       END IF
> +    END IF
> +  END SUBROUTINE parser_get_string
> +END MODULE cp_parser_methods
>

Re: Fix PR50260

[Michael Matz]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 8859 bytes --]

Hi,

On Fri, 2 Sep 2011, Richard Guenther wrote:

> > Currently regstrapping on x86_64-linux (without Ada).  Okay for trunk?
> 
> Ok.  Time to make get_var_ann private?

Yes.  Should the regstrap succeed on x86_64-linux I'll commit this 
variant of the patch (hunks in tree-flow.h, tree-flow-inline.h and 
tree-dfa.c are new, otherwise the same patch).


Ciao,
Michael.
-- 
	PR middle-end/50260
	* ipa-split.c (split_function): Call add_referenced_var.

	* tree-ssa-phiopt.c (cond_store_replacement): Don't call get_var_ann.
	(cond_if_else_store_replacement_1): Ditto.
	* tree-ssa-pre.c (get_representative_for): Ditto.
	(create_expression_by_pieces): Ditto.
	(insert_into_preds_of_block): Ditto.
	* tree-sra.c (create_access_replacement): Ditto.
	(get_replaced_param_substitute): Ditto.

	* tree-flow.h (get_var_ann): Don't declare.
	* tree-flow-inline.h (get_var_ann): Remove.
	(set_is_used): Use var_ann, not get_var_ann.
	* tree-dfa.c (add_referenced_var): Inline body of get_var_ann.

testsuite/
	* gfortran.fortran-torture/compile/pr50260.f90: New test.

Index: ipa-split.c
===================================================================
--- ipa-split.c	(revision 178408)
+++ ipa-split.c	(working copy)
@@ -988,6 +988,9 @@ split_function (struct split_point *spli
 	arg = gimple_default_def (cfun, parm);
 	if (!arg)
 	  {
+	    /* This parm wasn't used up to now, but is going to be used,
+	       hence register it.  */
+	    add_referenced_var (parm);
 	    arg = make_ssa_name (parm, gimple_build_nop ());
 	    set_default_def (parm, arg);
 	  }
Index: tree-ssa-phiopt.c
===================================================================
--- tree-ssa-phiopt.c	(revision 178408)
+++ tree-ssa-phiopt.c	(working copy)
@@ -1269,10 +1269,7 @@ cond_store_replacement (basic_block midd
   /* 2) Create a temporary where we can store the old content
         of the memory touched by the store, if we need to.  */
   if (!condstoretemp || TREE_TYPE (lhs) != TREE_TYPE (condstoretemp))
-    {
-      condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
-      get_var_ann (condstoretemp);
-    }
+    condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
   add_referenced_var (condstoretemp);
 
   /* 3) Insert a load from the memory of the store to the temporary
@@ -1355,10 +1352,7 @@ cond_if_else_store_replacement_1 (basic_
   /* 2) Create a temporary where we can store the old content
 	of the memory touched by the store, if we need to.  */
   if (!condstoretemp || TREE_TYPE (lhs) != TREE_TYPE (condstoretemp))
-    {
-      condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
-      get_var_ann (condstoretemp);
-    }
+    condstoretemp = create_tmp_reg (TREE_TYPE (lhs), "cstore");
   add_referenced_var (condstoretemp);
 
   /* 3) Create a PHI node at the join block, with one argument
Index: tree-ssa-pre.c
===================================================================
--- tree-ssa-pre.c	(revision 178408)
+++ tree-ssa-pre.c	(working copy)
@@ -1399,7 +1399,7 @@ get_representative_for (const pre_expr e
   if (!pretemp || exprtype != TREE_TYPE (pretemp))
     {
       pretemp = create_tmp_reg (exprtype, "pretmp");
-      get_var_ann (pretemp);
+      add_referenced_var (pretemp);
     }
 
   name = make_ssa_name (pretemp, gimple_build_nop ());
@@ -3178,10 +3178,7 @@ create_expression_by_pieces (basic_block
   /* Build and insert the assignment of the end result to the temporary
      that we will return.  */
   if (!pretemp || exprtype != TREE_TYPE (pretemp))
-    {
-      pretemp = create_tmp_reg (exprtype, "pretmp");
-      get_var_ann (pretemp);
-    }
+    pretemp = create_tmp_reg (exprtype, "pretmp");
 
   temp = pretemp;
   add_referenced_var (temp);
@@ -3441,10 +3438,7 @@ insert_into_preds_of_block (basic_block
 
   /* Now build a phi for the new variable.  */
   if (!prephitemp || TREE_TYPE (prephitemp) != type)
-    {
-      prephitemp = create_tmp_var (type, "prephitmp");
-      get_var_ann (prephitemp);
-    }
+    prephitemp = create_tmp_var (type, "prephitmp");
 
   temp = prephitemp;
   add_referenced_var (temp);
Index: tree-sra.c
===================================================================
--- tree-sra.c	(revision 178408)
+++ tree-sra.c	(working copy)
@@ -1825,7 +1825,6 @@ create_access_replacement (struct access
   tree repl;
 
   repl = create_tmp_var (access->type, "SR");
-  get_var_ann (repl);
   add_referenced_var (repl);
   if (rename)
     mark_sym_for_renaming (repl);
@@ -4106,7 +4105,6 @@ get_replaced_param_substitute (struct ip
       DECL_NAME (repl) = get_identifier (pretty_name);
       obstack_free (&name_obstack, pretty_name);
 
-      get_var_ann (repl);
       add_referenced_var (repl);
       adj->new_ssa_base = repl;
     }
Index: testsuite/gfortran.fortran-torture/compile/pr50260.f90
===================================================================
--- testsuite/gfortran.fortran-torture/compile/pr50260.f90	(revision 0)
+++ testsuite/gfortran.fortran-torture/compile/pr50260.f90	(revision 0)
@@ -0,0 +1,48 @@
+MODULE cp_parser_methods
+  INTEGER, PARAMETER :: default_string_length=80
+  INTEGER, PARAMETER :: default_path_length=250
+  TYPE ilist_type
+     LOGICAL                              :: in_use
+  END TYPE ilist_type
+  TYPE cp_parser_type
+     CHARACTER(LEN=default_path_length)             :: ifn
+     INTEGER                                        :: icol,icol1,icol2
+     TYPE(ilist_type), POINTER                      :: ilist
+  END TYPE cp_parser_type
+  TYPE cp_error_type
+  END TYPE cp_error_type
+CONTAINS
+  FUNCTION cts(i) RESULT(res)
+    CHARACTER(len=6)                         :: res
+  END FUNCTION cts
+  FUNCTION parser_location(parser,error) RESULT(res)
+    TYPE(cp_parser_type), POINTER            :: parser
+    TYPE(cp_error_type), INTENT(inout)       :: error
+    CHARACTER(len=default_path_length+default_string_length)       :: res
+    LOGICAL                                  :: failure
+    IF (.NOT. failure) THEN
+       res="file:'"//TRIM(parser%ifn)//"' line:"//cts(parser%icol)
+    END IF
+  END FUNCTION parser_location
+  SUBROUTINE parser_get_integer(parser,at_end, error)
+    TYPE(cp_parser_type), POINTER            :: parser
+    TYPE(cp_error_type), INTENT(inout)       :: error
+    LOGICAL                                  :: failure, my_at_end
+    IF (.NOT.failure) THEN
+       IF (.NOT.parser%ilist%in_use) THEN
+          CALL cp_assert("A"// TRIM(parser_location(parser,error)))
+       END IF
+    END IF
+  END SUBROUTINE parser_get_integer
+  SUBROUTINE parser_get_string(parser,at_end,error)
+    TYPE(cp_parser_type), POINTER            :: parser
+    LOGICAL, INTENT(out), OPTIONAL           :: at_end
+    TYPE(cp_error_type), INTENT(inout)       :: error
+    LOGICAL                                  :: failure, my_at_end
+    IF (.NOT.failure) THEN
+       IF (PRESENT(at_end)) THEN
+          CALL cp_assert("s"//TRIM(parser_location(parser,error)))
+       END IF
+    END IF
+  END SUBROUTINE parser_get_string
+END MODULE cp_parser_methods
Index: tree-flow.h
===================================================================
--- tree-flow.h	(revision 178408)
+++ tree-flow.h	(working copy)
@@ -278,7 +278,6 @@ typedef struct immediate_use_iterator_d
 typedef struct var_ann_d *var_ann_t;
 
 static inline var_ann_t var_ann (const_tree);
-static inline var_ann_t get_var_ann (tree);
 static inline void update_stmt (gimple);
 static inline int get_lineno (const_gimple);
 
Index: tree-flow-inline.h
===================================================================
--- tree-flow-inline.h	(revision 178408)
+++ tree-flow-inline.h	(working copy)
@@ -145,16 +145,6 @@ var_ann (const_tree t)
   return p ? *p : NULL;
 }
 
-/* Return the variable annotation for T, which must be a _DECL node.
-   Create the variable annotation if it doesn't exist.  */
-static inline var_ann_t
-get_var_ann (tree var)
-{
-  var_ann_t *p = DECL_VAR_ANN_PTR (var);
-  gcc_checking_assert (p);
-  return *p ? *p : create_var_ann (var);
-}
-
 /* Get the number of the next statement uid to be allocated.  */
 static inline unsigned int
 gimple_stmt_max_uid (struct function *fn)
@@ -568,7 +558,7 @@ phi_arg_index_from_use (use_operand_p us
 static inline void
 set_is_used (tree var)
 {
-  var_ann_t ann = get_var_ann (var);
+  var_ann_t ann = var_ann (var);
   ann->used = true;
 }
 
Index: tree-dfa.c
===================================================================
--- tree-dfa.c	(revision 178408)
+++ tree-dfa.c	(working copy)
@@ -580,8 +580,9 @@ set_default_def (tree var, tree def)
 bool
 add_referenced_var (tree var)
 {
-  get_var_ann (var);
   gcc_assert (DECL_P (var));
+  if (!*DECL_VAR_ANN_PTR (var))
+    create_var_ann (var);
 
   /* Insert VAR into the referenced_vars hash table if it isn't present.  */
   if (referenced_var_check_and_insert (var))

Re: Fix PR50260

[Michael Matz]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 688 bytes --]

Hi,

On Fri, 2 Sep 2011, Michael Matz wrote:

> Hi,
> 
> On Fri, 2 Sep 2011, Richard Guenther wrote:
> 
> > > Currently regstrapping on x86_64-linux (without Ada).  Okay for trunk?
> > 
> > Ok.  Time to make get_var_ann private?
> 
> Yes.  Should the regstrap succeed on x86_64-linux I'll commit this 
> variant of the patch (hunks in tree-flow.h, tree-flow-inline.h and 
> tree-dfa.c are new, otherwise the same patch).

As I feared the call to get_var_ann in set_is_used right now really is 
still needed, privatizing it hence isn't that straight forward.  For now 
I've committed the PR50260 fix without that cleanup as r178489 to fix 
bootstrap for some platforms.


Ciao,
Michael.

rfa: remove get_var_ann (was: Fix PR50260)

[Michael Matz]

Hi,

On Fri, 2 Sep 2011, Michael Matz wrote:

> > > Ok.  Time to make get_var_ann private?
> 
> As I feared the call to get_var_ann in set_is_used right now really is 
> still needed, privatizing it hence isn't that straight forward.

After pondering I concluded that it's not necessary to call set_is_used 
for variables without var annotation.  Those won't be in referenced_vars 
(or local_decls) and therefore also won't be removed from those lists no 
matter how hard we try.

Regstrapped on x86_64-linux (without Ada).  Okay for trunk?


Ciao,
Michael.
-- 
	* tree-flow.h (get_var_ann): Don't declare.
	* tree-flow-inline.h (get_var_ann): Remove.
	(set_is_used): Use var_ann, not get_var_ann.
	* tree-dfa.c (add_referenced_var): Inline body of get_var_ann.
	* tree-ssa-live.c (mark_all_vars_used_1): Check var_ann before
	calling set_is_used.

Index: tree-flow-inline.h
===================================================================
--- tree-flow-inline.h	(Revision 178488)
+++ tree-flow-inline.h	(Arbeitskopie)
@@ -145,16 +145,6 @@ var_ann (const_tree t)
   return p ? *p : NULL;
 }
 
-/* Return the variable annotation for T, which must be a _DECL node.
-   Create the variable annotation if it doesn't exist.  */
-static inline var_ann_t
-get_var_ann (tree var)
-{
-  var_ann_t *p = DECL_VAR_ANN_PTR (var);
-  gcc_checking_assert (p);
-  return *p ? *p : create_var_ann (var);
-}
-
 /* Get the number of the next statement uid to be allocated.  */
 static inline unsigned int
 gimple_stmt_max_uid (struct function *fn)
@@ -568,7 +558,7 @@ phi_arg_index_from_use (use_operand_p us
 static inline void
 set_is_used (tree var)
 {
-  var_ann_t ann = get_var_ann (var);
+  var_ann_t ann = var_ann (var);
   ann->used = true;
 }
 
Index: tree-flow.h
===================================================================
--- tree-flow.h	(Revision 178488)
+++ tree-flow.h	(Arbeitskopie)
@@ -278,7 +278,6 @@ typedef struct immediate_use_iterator_d
 typedef struct var_ann_d *var_ann_t;
 
 static inline var_ann_t var_ann (const_tree);
-static inline var_ann_t get_var_ann (tree);
 static inline void update_stmt (gimple);
 static inline int get_lineno (const_gimple);
 
Index: tree-dfa.c
===================================================================
--- tree-dfa.c	(Revision 178488)
+++ tree-dfa.c	(Arbeitskopie)
@@ -580,8 +580,9 @@ set_default_def (tree var, tree def)
 bool
 add_referenced_var (tree var)
 {
-  get_var_ann (var);
   gcc_assert (DECL_P (var));
+  if (!*DECL_VAR_ANN_PTR (var))
+    create_var_ann (var);
 
   /* Insert VAR into the referenced_vars hash table if it isn't present.  */
   if (referenced_var_check_and_insert (var))
Index: tree-ssa-live.c
===================================================================
--- tree-ssa-live.c	(Revision 178408)
+++ tree-ssa-live.c	(Arbeitskopie)
@@ -376,7 +376,10 @@ mark_all_vars_used_1 (tree *tp, int *wal
     {
       if (data != NULL && bitmap_clear_bit ((bitmap) data, DECL_UID (t)))
 	mark_all_vars_used (&DECL_INITIAL (t), data);
-      set_is_used (t);
+      /* If something has no annotation it's neither in referenced_vars,
+         nor in local_decls.  No use in marking it as used.  */
+      if (var_ann (t))
+	set_is_used (t);
     }
   /* remove_unused_scope_block_p requires information about labels
      which are not DECL_IGNORED_P to tell if they might be used in the IL.  */

Re: Fix PR50260

[Richard Guenther]

On Fri, Sep 2, 2011 at 8:40 PM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> On Fri, 2 Sep 2011, Michael Matz wrote:
>
>> Hi,
>>
>> On Fri, 2 Sep 2011, Richard Guenther wrote:
>>
>> > > Currently regstrapping on x86_64-linux (without Ada).  Okay for trunk?
>> >
>> > Ok.  Time to make get_var_ann private?
>>
>> Yes.  Should the regstrap succeed on x86_64-linux I'll commit this
>> variant of the patch (hunks in tree-flow.h, tree-flow-inline.h and
>> tree-dfa.c are new, otherwise the same patch).
>
> As I feared the call to get_var_ann in set_is_used right now really is
> still needed, privatizing it hence isn't that straight forward.  For now
> I've committed the PR50260 fix without that cleanup as r178489 to fix
> bootstrap for some platforms.

That's odd though - looking at the single caller it should only ever
be called for referenced vars - which means we miss a referenced var
somewhere.

Richard.

>
> Ciao,
> Michael.

Re: rfa: remove get_var_ann (was: Fix PR50260)

[Richard Guenther]

On Sat, Sep 3, 2011 at 3:51 AM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> On Fri, 2 Sep 2011, Michael Matz wrote:
>
>> > > Ok.  Time to make get_var_ann private?
>>
>> As I feared the call to get_var_ann in set_is_used right now really is
>> still needed, privatizing it hence isn't that straight forward.
>
> After pondering I concluded that it's not necessary to call set_is_used
> for variables without var annotation.  Those won't be in referenced_vars
> (or local_decls) and therefore also won't be removed from those lists no
> matter how hard we try.
>
> Regstrapped on x86_64-linux (without Ada).  Okay for trunk?

No.  We call mark_all_vars_used on trees contained in GIMPLE
non-debug statements.  All vars we can reach that way _have_
to be in the list of referenced vars.  That they are not is the bug.
So - can you investigate which var doesn't have an annotation
an why it isn't in referenced vars?

Richard.

>
> Ciao,
> Michael.
> --
>        * tree-flow.h (get_var_ann): Don't declare.
>        * tree-flow-inline.h (get_var_ann): Remove.
>        (set_is_used): Use var_ann, not get_var_ann.
>        * tree-dfa.c (add_referenced_var): Inline body of get_var_ann.
>        * tree-ssa-live.c (mark_all_vars_used_1): Check var_ann before
>        calling set_is_used.
>
> Index: tree-flow-inline.h
> ===================================================================
> --- tree-flow-inline.h  (Revision 178488)
> +++ tree-flow-inline.h  (Arbeitskopie)
> @@ -145,16 +145,6 @@ var_ann (const_tree t)
>   return p ? *p : NULL;
>  }
>
> -/* Return the variable annotation for T, which must be a _DECL node.
> -   Create the variable annotation if it doesn't exist.  */
> -static inline var_ann_t
> -get_var_ann (tree var)
> -{
> -  var_ann_t *p = DECL_VAR_ANN_PTR (var);
> -  gcc_checking_assert (p);
> -  return *p ? *p : create_var_ann (var);
> -}
> -
>  /* Get the number of the next statement uid to be allocated.  */
>  static inline unsigned int
>  gimple_stmt_max_uid (struct function *fn)
> @@ -568,7 +558,7 @@ phi_arg_index_from_use (use_operand_p us
>  static inline void
>  set_is_used (tree var)
>  {
> -  var_ann_t ann = get_var_ann (var);
> +  var_ann_t ann = var_ann (var);
>   ann->used = true;
>  }
>
> Index: tree-flow.h
> ===================================================================
> --- tree-flow.h (Revision 178488)
> +++ tree-flow.h (Arbeitskopie)
> @@ -278,7 +278,6 @@ typedef struct immediate_use_iterator_d
>  typedef struct var_ann_d *var_ann_t;
>
>  static inline var_ann_t var_ann (const_tree);
> -static inline var_ann_t get_var_ann (tree);
>  static inline void update_stmt (gimple);
>  static inline int get_lineno (const_gimple);
>
> Index: tree-dfa.c
> ===================================================================
> --- tree-dfa.c  (Revision 178488)
> +++ tree-dfa.c  (Arbeitskopie)
> @@ -580,8 +580,9 @@ set_default_def (tree var, tree def)
>  bool
>  add_referenced_var (tree var)
>  {
> -  get_var_ann (var);
>   gcc_assert (DECL_P (var));
> +  if (!*DECL_VAR_ANN_PTR (var))
> +    create_var_ann (var);
>
>   /* Insert VAR into the referenced_vars hash table if it isn't present.  */
>   if (referenced_var_check_and_insert (var))
> Index: tree-ssa-live.c
> ===================================================================
> --- tree-ssa-live.c     (Revision 178408)
> +++ tree-ssa-live.c     (Arbeitskopie)
> @@ -376,7 +376,10 @@ mark_all_vars_used_1 (tree *tp, int *wal
>     {
>       if (data != NULL && bitmap_clear_bit ((bitmap) data, DECL_UID (t)))
>        mark_all_vars_used (&DECL_INITIAL (t), data);
> -      set_is_used (t);
> +      /* If something has no annotation it's neither in referenced_vars,
> +         nor in local_decls.  No use in marking it as used.  */
> +      if (var_ann (t))
> +       set_is_used (t);
>     }
>   /* remove_unused_scope_block_p requires information about labels
>      which are not DECL_IGNORED_P to tell if they might be used in the IL.  */
>

Re: rfa: remove get_var_ann (was: Fix PR50260)

[Michael Matz]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1520 bytes --]

Hi,

On Sat, 3 Sep 2011, Richard Guenther wrote:

> >> As I feared the call to get_var_ann in set_is_used right now really 
> >> is still needed, privatizing it hence isn't that straight forward.
> >
> > After pondering I concluded that it's not necessary to call 
> > set_is_used for variables without var annotation.  Those won't be in 
> > referenced_vars (or local_decls) and therefore also won't be removed 
> > from those lists no matter how hard we try.
> >
> > Regstrapped on x86_64-linux (without Ada).  Okay for trunk?
> 
> No.  We call mark_all_vars_used on trees contained in GIMPLE non-debug 
> statements.  All vars we can reach that way _have_ to be in the list of 
> referenced vars.

Sometimes global variables (non-auto vars with context != cfun) can be 
missing from referenced_vars.  In the case where we hit bugs with this 
it's the non-local variables generated for profile counters.  I can add 
the respective add_referenced_var calls for that, but I'm not sure I see 
the point.  That of course comes back to our old discussion for what 
purposes referenced_vars actually is used.  I looked at all users and I 
think that the global counters missing from referenced_vars is harmless.

OTOH it's a nice invariant that can actually be checked for (that all 
reachable vars whatsoever have to be in referenced_vars), so I'm going to 
do that.

> That they are not is the bug. So - can you investigate 
> which var doesn't have an annotation an why it isn't in referenced vars?


Ciao,
Michael.

Re: rfa: remove get_var_ann (was: Fix PR50260)

[Richard Guenther]

On Sat, Sep 3, 2011 at 5:31 PM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> On Sat, 3 Sep 2011, Richard Guenther wrote:
>
>> >> As I feared the call to get_var_ann in set_is_used right now really
>> >> is still needed, privatizing it hence isn't that straight forward.
>> >
>> > After pondering I concluded that it's not necessary to call
>> > set_is_used for variables without var annotation.  Those won't be in
>> > referenced_vars (or local_decls) and therefore also won't be removed
>> > from those lists no matter how hard we try.
>> >
>> > Regstrapped on x86_64-linux (without Ada).  Okay for trunk?
>>
>> No.  We call mark_all_vars_used on trees contained in GIMPLE non-debug
>> statements.  All vars we can reach that way _have_ to be in the list of
>> referenced vars.
>
> Sometimes global variables (non-auto vars with context != cfun) can be
> missing from referenced_vars.  In the case where we hit bugs with this
> it's the non-local variables generated for profile counters.  I can add
> the respective add_referenced_var calls for that, but I'm not sure I see
> the point.  That of course comes back to our old discussion for what
> purposes referenced_vars actually is used.  I looked at all users and I
> think that the global counters missing from referenced_vars is harmless.
>
> OTOH it's a nice invariant that can actually be checked for (that all
> reachable vars whatsoever have to be in referenced_vars), so I'm going to
> do that.

Yes, until we get rid of referenced_vars (which we still should do at
some point...)
that's the best.  IIRC we have some verification code even, and wonder why it
doesn't trigger.

Richard.

>> That they are not is the bug. So - can you investigate
>> which var doesn't have an annotation an why it isn't in referenced vars?
>
>
> Ciao,
> Michael.

Re: rfa: remove get_var_ann (was: Fix PR50260)

[Michael Matz]

Hi,

On Sat, 3 Sep 2011, Richard Guenther wrote:

> > OTOH it's a nice invariant that can actually be checked for (that all 
> > reachable vars whatsoever have to be in referenced_vars), so I'm going 
> > to do that.
> 
> Yes, until we get rid of referenced_vars (which we still should do at 
> some point...) that's the best.

Okay, like so then.  Regstrapped on x86_64-linux.  (Note that sometimes I 
use add_referenced_vars, and sometimes find_referenced_vars_in, the latter 
when I would have to add several add_referenced_vars for one statement).

> IIRC we have some verification code even, and wonder why it doesn't 
> trigger.

Nope, we don't.  But with the patch we segfault in case this happens 
again, which is good enough checking for me.


Ciao,
Michael.
----------------
	* tree-flow.h (get_var_ann): Don't declare.
	* tree-flow-inline.h (get_var_ann): Remove.
	(set_is_used): Use var_ann, not get_var_ann.
	* tree-dfa.c (add_referenced_var): Inline body of get_var_ann.
	* tree-profile.c (gimple_gen_edge_profiler): Call
	find_referenced_var_in.
	(gimple_gen_interval_profiler): Ditto.
	(gimple_gen_pow2_profiler): Ditto.
	(gimple_gen_one_value_profiler): Ditto.
	(gimple_gen_average_profiler): Ditto.
	(gimple_gen_ior_profiler): Ditto.
	(gimple_gen_ic_profiler): Ditto plus call add_referenced_var.
	(gimple_gen_ic_func_profiler): Call add_referenced_var.
	* tree-mudflap.c (execute_mudflap_function_ops): Call
	add_referenced_var.

Index: tree-flow.h
===================================================================
--- tree-flow.h	(revision 178488)
+++ tree-flow.h	(working copy)
@@ -278,7 +278,6 @@ typedef struct immediate_use_iterator_d
 typedef struct var_ann_d *var_ann_t;
 
 static inline var_ann_t var_ann (const_tree);
-static inline var_ann_t get_var_ann (tree);
 static inline void update_stmt (gimple);
 static inline int get_lineno (const_gimple);
 
Index: tree-flow-inline.h
===================================================================
--- tree-flow-inline.h	(revision 178488)
+++ tree-flow-inline.h	(working copy)
@@ -145,16 +145,6 @@ var_ann (const_tree t)
   return p ? *p : NULL;
 }
 
-/* Return the variable annotation for T, which must be a _DECL node.
-   Create the variable annotation if it doesn't exist.  */
-static inline var_ann_t
-get_var_ann (tree var)
-{
-  var_ann_t *p = DECL_VAR_ANN_PTR (var);
-  gcc_checking_assert (p);
-  return *p ? *p : create_var_ann (var);
-}
-
 /* Get the number of the next statement uid to be allocated.  */
 static inline unsigned int
 gimple_stmt_max_uid (struct function *fn)
@@ -568,7 +558,7 @@ phi_arg_index_from_use (use_operand_p us
 static inline void
 set_is_used (tree var)
 {
-  var_ann_t ann = get_var_ann (var);
+  var_ann_t ann = var_ann (var);
   ann->used = true;
 }
 
Index: tree-dfa.c
===================================================================
--- tree-dfa.c	(revision 178488)
+++ tree-dfa.c	(working copy)
@@ -580,8 +580,9 @@ set_default_def (tree var, tree def)
 bool
 add_referenced_var (tree var)
 {
-  get_var_ann (var);
   gcc_assert (DECL_P (var));
+  if (!*DECL_VAR_ANN_PTR (var))
+    create_var_ann (var);
 
   /* Insert VAR into the referenced_vars hash table if it isn't present.  */
   if (referenced_var_check_and_insert (var))
Index: tree-profile.c
===================================================================
--- tree-profile.c	(revision 178408)
+++ tree-profile.c	(working copy)
@@ -224,6 +224,7 @@ gimple_gen_edge_profiler (int edgeno, ed
   one = build_int_cst (gcov_type_node, 1);
   stmt1 = gimple_build_assign (gcov_type_tmp_var, ref);
   gimple_assign_set_lhs (stmt1, make_ssa_name (gcov_type_tmp_var, stmt1));
+  find_referenced_vars_in (stmt1);
   stmt2 = gimple_build_assign_with_ops (PLUS_EXPR, gcov_type_tmp_var,
 					gimple_assign_lhs (stmt1), one);
   gimple_assign_set_lhs (stmt2, make_ssa_name (gcov_type_tmp_var, stmt2));
@@ -270,6 +271,7 @@ gimple_gen_interval_profiler (histogram_
   val = prepare_instrumented_value (&gsi, value);
   call = gimple_build_call (tree_interval_profiler_fn, 4,
 			    ref_ptr, val, start, steps);
+  find_referenced_vars_in (call);
   gsi_insert_before (&gsi, call, GSI_NEW_STMT);
 }
 
@@ -290,6 +292,7 @@ gimple_gen_pow2_profiler (histogram_valu
 				      true, NULL_TREE, true, GSI_SAME_STMT);
   val = prepare_instrumented_value (&gsi, value);
   call = gimple_build_call (tree_pow2_profiler_fn, 2, ref_ptr, val);
+  find_referenced_vars_in (call);
   gsi_insert_before (&gsi, call, GSI_NEW_STMT);
 }
 
@@ -310,6 +313,7 @@ gimple_gen_one_value_profiler (histogram
 				      true, NULL_TREE, true, GSI_SAME_STMT);
   val = prepare_instrumented_value (&gsi, value);
   call = gimple_build_call (tree_one_value_profiler_fn, 2, ref_ptr, val);
+  find_referenced_vars_in (call);
   gsi_insert_before (&gsi, call, GSI_NEW_STMT);
 }
 
@@ -340,9 +344,12 @@ gimple_gen_ic_profiler (histogram_value
 
   tmp1 = create_tmp_reg (ptr_void, "PROF");
   stmt1 = gimple_build_assign (ic_gcov_type_ptr_var, ref_ptr);
+  find_referenced_vars_in (stmt1);
   stmt2 = gimple_build_assign (tmp1, unshare_expr (value->hvalue.value));
   gimple_assign_set_lhs (stmt2, make_ssa_name (tmp1, stmt2));
+  find_referenced_vars_in (stmt2);
   stmt3 = gimple_build_assign (ic_void_ptr_var, gimple_assign_lhs (stmt2));
+  add_referenced_var (ic_void_ptr_var);
 
   gsi_insert_before (&gsi, stmt1, GSI_SAME_STMT);
   gsi_insert_before (&gsi, stmt2, GSI_SAME_STMT);
@@ -378,9 +385,11 @@ gimple_gen_ic_func_profiler (void)
   counter_ptr = force_gimple_operand_gsi (&gsi, ic_gcov_type_ptr_var,
 					  true, NULL_TREE, true,
 					  GSI_SAME_STMT);
+  add_referenced_var (ic_gcov_type_ptr_var);
   ptr_var = force_gimple_operand_gsi (&gsi, ic_void_ptr_var,
 				      true, NULL_TREE, true,
 				      GSI_SAME_STMT);
+  add_referenced_var (ic_void_ptr_var);
   tree_uid = build_int_cst (gcov_type_node, current_function_funcdef_no);
   stmt1 = gimple_build_call (tree_indirect_call_profiler_fn, 4,
 			     counter_ptr, tree_uid, cur_func, ptr_var);
@@ -429,6 +438,7 @@ gimple_gen_average_profiler (histogram_v
 				      true, GSI_SAME_STMT);
   val = prepare_instrumented_value (&gsi, value);
   call = gimple_build_call (tree_average_profiler_fn, 2, ref_ptr, val);
+  find_referenced_vars_in (call);
   gsi_insert_before (&gsi, call, GSI_NEW_STMT);
 }
 
@@ -449,6 +459,7 @@ gimple_gen_ior_profiler (histogram_value
 				      true, NULL_TREE, true, GSI_SAME_STMT);
   val = prepare_instrumented_value (&gsi, value);
   call = gimple_build_call (tree_ior_profiler_fn, 2, ref_ptr, val);
+  find_referenced_vars_in (call);
   gsi_insert_before (&gsi, call, GSI_NEW_STMT);
 }
 
Index: tree-mudflap.c
===================================================================
--- tree-mudflap.c	(revision 178600)
+++ tree-mudflap.c	(working copy)
@@ -418,6 +418,10 @@ execute_mudflap_function_ops (void)
 
   push_gimplify_context (&gctx);
 
+  add_referenced_var (mf_cache_array_decl);
+  add_referenced_var (mf_cache_shift_decl);
+  add_referenced_var (mf_cache_mask_decl);
+
   /* In multithreaded mode, don't cache the lookup cache parameters.  */
   if (! flag_mudflap_threads)
     mf_decl_cache_locals ();

Re: rfa: remove get_var_ann (was: Fix PR50260)

[Richard Guenther]

On Thu, Oct 6, 2011 at 4:59 PM, Michael Matz <matz@suse.de> wrote:
> Hi,
>
> On Sat, 3 Sep 2011, Richard Guenther wrote:
>
>> > OTOH it's a nice invariant that can actually be checked for (that all
>> > reachable vars whatsoever have to be in referenced_vars), so I'm going
>> > to do that.
>>
>> Yes, until we get rid of referenced_vars (which we still should do at
>> some point...) that's the best.
>
> Okay, like so then.  Regstrapped on x86_64-linux.  (Note that sometimes I
> use add_referenced_vars, and sometimes find_referenced_vars_in, the latter
> when I would have to add several add_referenced_vars for one statement).
>
>> IIRC we have some verification code even, and wonder why it doesn't
>> trigger.
>
> Nope, we don't.  But with the patch we segfault in case this happens
> again, which is good enough checking for me.

Ok.

Thanks,
Richard.

>
> Ciao,
> Michael.
> ----------------
>        * tree-flow.h (get_var_ann): Don't declare.
>        * tree-flow-inline.h (get_var_ann): Remove.
>        (set_is_used): Use var_ann, not get_var_ann.
>        * tree-dfa.c (add_referenced_var): Inline body of get_var_ann.
>        * tree-profile.c (gimple_gen_edge_profiler): Call
>        find_referenced_var_in.
>        (gimple_gen_interval_profiler): Ditto.
>        (gimple_gen_pow2_profiler): Ditto.
>        (gimple_gen_one_value_profiler): Ditto.
>        (gimple_gen_average_profiler): Ditto.
>        (gimple_gen_ior_profiler): Ditto.
>        (gimple_gen_ic_profiler): Ditto plus call add_referenced_var.
>        (gimple_gen_ic_func_profiler): Call add_referenced_var.
>        * tree-mudflap.c (execute_mudflap_function_ops): Call
>        add_referenced_var.
>
> Index: tree-flow.h
> ===================================================================
> --- tree-flow.h (revision 178488)
> +++ tree-flow.h (working copy)
> @@ -278,7 +278,6 @@ typedef struct immediate_use_iterator_d
>  typedef struct var_ann_d *var_ann_t;
>
>  static inline var_ann_t var_ann (const_tree);
> -static inline var_ann_t get_var_ann (tree);
>  static inline void update_stmt (gimple);
>  static inline int get_lineno (const_gimple);
>
> Index: tree-flow-inline.h
> ===================================================================
> --- tree-flow-inline.h  (revision 178488)
> +++ tree-flow-inline.h  (working copy)
> @@ -145,16 +145,6 @@ var_ann (const_tree t)
>   return p ? *p : NULL;
>  }
>
> -/* Return the variable annotation for T, which must be a _DECL node.
> -   Create the variable annotation if it doesn't exist.  */
> -static inline var_ann_t
> -get_var_ann (tree var)
> -{
> -  var_ann_t *p = DECL_VAR_ANN_PTR (var);
> -  gcc_checking_assert (p);
> -  return *p ? *p : create_var_ann (var);
> -}
> -
>  /* Get the number of the next statement uid to be allocated.  */
>  static inline unsigned int
>  gimple_stmt_max_uid (struct function *fn)
> @@ -568,7 +558,7 @@ phi_arg_index_from_use (use_operand_p us
>  static inline void
>  set_is_used (tree var)
>  {
> -  var_ann_t ann = get_var_ann (var);
> +  var_ann_t ann = var_ann (var);
>   ann->used = true;
>  }
>
> Index: tree-dfa.c
> ===================================================================
> --- tree-dfa.c  (revision 178488)
> +++ tree-dfa.c  (working copy)
> @@ -580,8 +580,9 @@ set_default_def (tree var, tree def)
>  bool
>  add_referenced_var (tree var)
>  {
> -  get_var_ann (var);
>   gcc_assert (DECL_P (var));
> +  if (!*DECL_VAR_ANN_PTR (var))
> +    create_var_ann (var);
>
>   /* Insert VAR into the referenced_vars hash table if it isn't present.  */
>   if (referenced_var_check_and_insert (var))
> Index: tree-profile.c
> ===================================================================
> --- tree-profile.c      (revision 178408)
> +++ tree-profile.c      (working copy)
> @@ -224,6 +224,7 @@ gimple_gen_edge_profiler (int edgeno, ed
>   one = build_int_cst (gcov_type_node, 1);
>   stmt1 = gimple_build_assign (gcov_type_tmp_var, ref);
>   gimple_assign_set_lhs (stmt1, make_ssa_name (gcov_type_tmp_var, stmt1));
> +  find_referenced_vars_in (stmt1);
>   stmt2 = gimple_build_assign_with_ops (PLUS_EXPR, gcov_type_tmp_var,
>                                        gimple_assign_lhs (stmt1), one);
>   gimple_assign_set_lhs (stmt2, make_ssa_name (gcov_type_tmp_var, stmt2));
> @@ -270,6 +271,7 @@ gimple_gen_interval_profiler (histogram_
>   val = prepare_instrumented_value (&gsi, value);
>   call = gimple_build_call (tree_interval_profiler_fn, 4,
>                            ref_ptr, val, start, steps);
> +  find_referenced_vars_in (call);
>   gsi_insert_before (&gsi, call, GSI_NEW_STMT);
>  }
>
> @@ -290,6 +292,7 @@ gimple_gen_pow2_profiler (histogram_valu
>                                      true, NULL_TREE, true, GSI_SAME_STMT);
>   val = prepare_instrumented_value (&gsi, value);
>   call = gimple_build_call (tree_pow2_profiler_fn, 2, ref_ptr, val);
> +  find_referenced_vars_in (call);
>   gsi_insert_before (&gsi, call, GSI_NEW_STMT);
>  }
>
> @@ -310,6 +313,7 @@ gimple_gen_one_value_profiler (histogram
>                                      true, NULL_TREE, true, GSI_SAME_STMT);
>   val = prepare_instrumented_value (&gsi, value);
>   call = gimple_build_call (tree_one_value_profiler_fn, 2, ref_ptr, val);
> +  find_referenced_vars_in (call);
>   gsi_insert_before (&gsi, call, GSI_NEW_STMT);
>  }
>
> @@ -340,9 +344,12 @@ gimple_gen_ic_profiler (histogram_value
>
>   tmp1 = create_tmp_reg (ptr_void, "PROF");
>   stmt1 = gimple_build_assign (ic_gcov_type_ptr_var, ref_ptr);
> +  find_referenced_vars_in (stmt1);
>   stmt2 = gimple_build_assign (tmp1, unshare_expr (value->hvalue.value));
>   gimple_assign_set_lhs (stmt2, make_ssa_name (tmp1, stmt2));
> +  find_referenced_vars_in (stmt2);
>   stmt3 = gimple_build_assign (ic_void_ptr_var, gimple_assign_lhs (stmt2));
> +  add_referenced_var (ic_void_ptr_var);
>
>   gsi_insert_before (&gsi, stmt1, GSI_SAME_STMT);
>   gsi_insert_before (&gsi, stmt2, GSI_SAME_STMT);
> @@ -378,9 +385,11 @@ gimple_gen_ic_func_profiler (void)
>   counter_ptr = force_gimple_operand_gsi (&gsi, ic_gcov_type_ptr_var,
>                                          true, NULL_TREE, true,
>                                          GSI_SAME_STMT);
> +  add_referenced_var (ic_gcov_type_ptr_var);
>   ptr_var = force_gimple_operand_gsi (&gsi, ic_void_ptr_var,
>                                      true, NULL_TREE, true,
>                                      GSI_SAME_STMT);
> +  add_referenced_var (ic_void_ptr_var);
>   tree_uid = build_int_cst (gcov_type_node, current_function_funcdef_no);
>   stmt1 = gimple_build_call (tree_indirect_call_profiler_fn, 4,
>                             counter_ptr, tree_uid, cur_func, ptr_var);
> @@ -429,6 +438,7 @@ gimple_gen_average_profiler (histogram_v
>                                      true, GSI_SAME_STMT);
>   val = prepare_instrumented_value (&gsi, value);
>   call = gimple_build_call (tree_average_profiler_fn, 2, ref_ptr, val);
> +  find_referenced_vars_in (call);
>   gsi_insert_before (&gsi, call, GSI_NEW_STMT);
>  }
>
> @@ -449,6 +459,7 @@ gimple_gen_ior_profiler (histogram_value
>                                      true, NULL_TREE, true, GSI_SAME_STMT);
>   val = prepare_instrumented_value (&gsi, value);
>   call = gimple_build_call (tree_ior_profiler_fn, 2, ref_ptr, val);
> +  find_referenced_vars_in (call);
>   gsi_insert_before (&gsi, call, GSI_NEW_STMT);
>  }
>
> Index: tree-mudflap.c
> ===================================================================
> --- tree-mudflap.c      (revision 178600)
> +++ tree-mudflap.c      (working copy)
> @@ -418,6 +418,10 @@ execute_mudflap_function_ops (void)
>
>   push_gimplify_context (&gctx);
>
> +  add_referenced_var (mf_cache_array_decl);
> +  add_referenced_var (mf_cache_shift_decl);
> +  add_referenced_var (mf_cache_mask_decl);
> +
>   /* In multithreaded mode, don't cache the lookup cache parameters.  */
>   if (! flag_mudflap_threads)
>     mf_decl_cache_locals ();
>