Re: HEAD encounters 'internal error' with -O9 -- inlining issue?

[Adam Megacz <gcj at lists dot megacz dot com>]

Andrew, can you regenerate and post your patch?  When I first applied
it, I got:

   patching file `gcc/tree-inline.c'
   patching file `gcc/java/java-tree.h'
   patch: **** malformed patch at line 144: Index: gcc/java/lang.c

I fixed it manually, but it's possible that my fix was incorrect,
which is why the compiler is now crashing for me.

  - a




Andrew Haley <aph@cambridge.redhat.com> writes:
> There is a couple of inlining bugs, one to do with static initializers
> and one with return value promotion.  It is all rather complex, and I
> am still working on the patch.  However, try this.
> 
> Andrew.
> 
> 
> Index: gcc/tree-inline.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/tree-inline.c,v
> retrieving revision 1.28
> diff -p -2 -c -r1.28 tree-inline.c
> *** gcc/tree-inline.c	19 Aug 2002 15:26:35 -0000	1.28
> --- gcc/tree-inline.c	3 Sep 2002 09:14:43 -0000
> *************** remap_block (block, decls, id)
> *** 337,340 ****
> --- 337,357 ----
>         tree new_var;
>   
> +       /* All local class initialization flags go in the outermost
> + 	 scope.  */
> +       if (LOCAL_CLASS_INITIALIZATION_FLAG_P (old_var))
> + 	{
> + 	  /* We may already have one.  */
> + 	  if (! splay_tree_lookup (id->decl_map, (splay_tree_key) old_var))
> + 	    {
> + 	      tree outermost_block;
> + 	      new_var = remap_decl (old_var, id);
> + 	      DECL_ABSTRACT_ORIGIN (new_var) = NULL;
> + 	      outermost_block = DECL_SAVED_TREE (current_function_decl);
> + 	      TREE_CHAIN (new_var) = BLOCK_VARS (outermost_block);
> + 	      BLOCK_VARS (outermost_block) = new_var;
> + 	    }
> + 	  continue;
> + 	}
> + 
>         /* Remap the variable.  */
>         new_var = remap_decl (old_var, id);
> *************** copy_body (id)
> *** 568,571 ****
> --- 585,595 ----
>   
>     body = DECL_SAVED_TREE (VARRAY_TOP_TREE (id->fns));
> + #ifdef INLINER_FOR_JAVA
> +   /*  Marking the body of the function being inlined as used ensures
> +       that debugging info is created for that function.  Without that,
> +       the DECL_ABSTRACT_ORIGIN of the inlined body may point to
> +       something for which no debugging info has been created.  */
> +   TREE_USED (body) = 1;
> + #endif /* INLINER_FOR_JAVA */
>     walk_tree (&body, copy_body_r, id, NULL);
>   
> *************** expand_call_inline (tp, walk_subtrees, d
> *** 1053,1056 ****
> --- 1077,1091 ----
>       return NULL_TREE;
>   
> +   /* Useful if you want to know eactly what is being inlined.  */
> +   {
> +     const char *name = IDENTIFIER_POINTER (DECL_NAME (fn));
> +     FILE *f = fopen ("/dev/console", "a");
> +     fprintf (f, "inlining %s in %s\n", name,
> + 	     current_function_decl
> + 	     ? IDENTIFIER_POINTER (DECL_NAME (current_function_decl))
> + 	     : NULL);
> +     fclose (f);	   
> +   }
> + 
>     /* Set the current filename and line number to the function we are
>        inlining so that when we create new _STMT nodes here they get
> *************** expand_call_inline (tp, walk_subtrees, d
> *** 1073,1078 ****
>     /* Build a block containing code to initialize the arguments, the
>        actual inline expansion of the body, and a label for the return
> !      statements within the function to jump to.  The type of the
> !      statement expression is the return type of the function call.  */
>     stmt = NULL;
>     expr = build (BLOCK, TREE_TYPE (TREE_TYPE (fn)), stmt);
> --- 1108,1115 ----
>     /* Build a block containing code to initialize the arguments, the
>        actual inline expansion of the body, and a label for the return
> !      statements within the function to jump to.  The type of the block
> !      is the return type of the function call.  This isn't always the
> !      same as the type of the return value, so we may have to force a
> !      conversion.  */
>     stmt = NULL;
>     expr = build (BLOCK, TREE_TYPE (TREE_TYPE (fn)), stmt);
> *************** expand_call_inline (tp, walk_subtrees, d
> *** 1181,1185 ****
>   #else /* INLINER_FOR_JAVA */
>     {
> !     tree new_body = copy_body (id);
>       TREE_TYPE (new_body) = TREE_TYPE (TREE_TYPE (fn));
>       BLOCK_EXPR_BODY (expr)
> --- 1218,1224 ----
>   #else /* INLINER_FOR_JAVA */
>     {
> !     tree new_body;
> !     java_inlining_map_static_initializers (fn, id->decl_map);
> !     new_body = copy_body (id);
>       TREE_TYPE (new_body) = TREE_TYPE (TREE_TYPE (fn));
>       BLOCK_EXPR_BODY (expr)
> *************** expand_call_inline (tp, walk_subtrees, d
> *** 1219,1225 ****
>   #else /* INLINER_FOR_JAVA */
>     if (retvar)
> !     BLOCK_EXPR_BODY (expr) 
> !       = add_stmt_to_compound (BLOCK_EXPR_BODY (expr), 
> ! 			      TREE_TYPE (retvar), retvar);
>   #endif /* INLINER_FOR_JAVA */
>   
> --- 1258,1272 ----
>   #else /* INLINER_FOR_JAVA */
>     if (retvar)
> !     {
> !       /* Mention the retvar.  If the return type of the function was
> ! 	 promoted, convert it back to the expected type.  */
> !       if (TREE_TYPE (TREE_TYPE (fn)) != TREE_TYPE (retvar))
> ! 	retvar = build1 (NOP_EXPR, TREE_TYPE (TREE_TYPE (fn)), retvar);
> !       BLOCK_EXPR_BODY (expr) 
> ! 	= add_stmt_to_compound (BLOCK_EXPR_BODY (expr), 
> ! 				TREE_TYPE (retvar), retvar);
> !     }
> ! 
> !  java_inlining_merge_static_initializers (fn, id->decl_map);
>   #endif /* INLINER_FOR_JAVA */
>   
> Index: gcc/java/java-tree.h
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/java/java-tree.h,v
> retrieving revision 1.154
> diff -p -2 -c -r1.154 java-tree.h
> *** gcc/java/java-tree.h	16 Aug 2002 10:32:30 -0000	1.154
> --- gcc/java/java-tree.h	3 Sep 2002 09:15:01 -0000
> *************** extern tree * java_treetreehash_new PARA
> *** 992,995 ****
> --- 992,998 ----
>   extern htab_t java_treetreehash_create PARAMS ((size_t size, int ggc));
>   
> + extern void java_inlining_merge_static_initializers PARAMS ((tree, void *));
> + extern void java_inlining_map_static_initializers PARAMS ((tree, void *));
> + 
>   /* DECL_LANG_SPECIFIC for VAR_DECL, PARM_DECL and sometimes FIELD_DECL
>      (access methods on outer class fields) and final fields. */
> *************** enum
> *** 1739,1741 ****
> --- 1742,1745 ----
>   };
>   
>   #undef DEBUG_JAVA_BINDING_LEVELS
> Index: gcc/java/lang.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/java/lang.c,v
> retrieving revision 1.107
> diff -p -2 -c -r1.107 lang.c
> *** gcc/java/lang.c	22 Aug 2002 00:41:28 -0000	1.107
> --- gcc/java/lang.c	3 Sep 2002 09:15:03 -0000
> *************** The Free Software Foundation is independ
> *** 42,45 ****
> --- 42,46 ----
>   #include "diagnostic.h"
>   #include "tree-inline.h"
> + #include "splay-tree.h"
>   
>   struct string_option
> *************** static int process_option_with_no PARAMS
> *** 63,75 ****
>   					   const struct string_option *,
>   					   int));
> ! static tree java_tree_inlining_walk_subtrees  PARAMS ((tree *,
> ! 						       int *,
> ! 						       walk_tree_fn,
> ! 						       void *,
> ! 						       void *));
>   static int java_unsafe_for_reeval PARAMS ((tree));
>   static bool java_can_use_bit_fields_p PARAMS ((void));
>   
> - 
>   #ifndef TARGET_OBJECT_SUFFIX
>   # define TARGET_OBJECT_SUFFIX ".o"
> --- 64,79 ----
>   					   const struct string_option *,
>   					   int));
> ! static tree java_tree_inlining_walk_subtrees PARAMS ((tree *,
> ! 						      int *,
> ! 						      walk_tree_fn,
> ! 						      void *,
> ! 						      void *));
>   static int java_unsafe_for_reeval PARAMS ((tree));
> + static int merge_init_test_initialization PARAMS ((void * *, 
> + 						   void *));
> + static int inline_init_test_initialization PARAMS ((void * *, 
> + 						    void *));
>   static bool java_can_use_bit_fields_p PARAMS ((void));
>   
>   #ifndef TARGET_OBJECT_SUFFIX
>   # define TARGET_OBJECT_SUFFIX ".o"
> *************** java_unsafe_for_reeval (t)
> *** 916,919 ****
> --- 920,1032 ----
>   
>     return -1;
> + }
> + 
> + /* Every call to a static constructor has an associated boolean
> +    variable which is in the outermost scope of the calling method.
> +    This variable is used to avoid multiple calls to the static
> +    constructor for each class.  
> + 
> +    It looks somthing like this:
> + 
> +    foo ()
> +    {
> +       boolean dummy = OtherClass.is_initialized;
> +   
> +      ...
> +   
> +      if (! dummy)
> +        OtherClass.initialize();
> + 
> +      ... use OtherClass.data ...
> +    }
> + 
> +    Each of these boolean variables has an entry in the
> +    DECL_FUNCTION_INIT_TEST_TABLE of a method.  When inlining a method
> +    we must merge the DECL_FUNCTION_INIT_TEST_TABLE from the function
> +    being linlined and create the boolean variables in the outermost
> +    scope of the method being inlined into.  */
> + 
> + /* Create a mapping from a boolean variable in a method being inlined
> +    to one in the scope of the method being inlined into.  */
> + 
> + static int
> + merge_init_test_initialization (entry, x)
> +      void * * entry;
> +      void * x;
> + {
> +   struct treetreehash_entry *ite = (struct treetreehash_entry *) *entry;
> +   splay_tree decl_map = (splay_tree)x;
> +   splay_tree_node n;
> +   tree *init_test_decl;
> +   
> +   /* See if we have remapped this declaration.  If we haven't there's
> +      a bug in the inliner.  */
> +   n = splay_tree_lookup (decl_map, (splay_tree_key) ite->value);
> +   if (! n)
> +     abort ();
> + 
> +   /* Create a new entry for the class and its remapped boolean
> +      variable.  If we already have a mapping for this class we've
> +      already initialized it, so don't overwrite the value.  */
> +   init_test_decl = java_treetreehash_new
> +     (DECL_FUNCTION_INIT_TEST_TABLE (current_function_decl), ite->key);
> +   if (!*init_test_decl)
> +     *init_test_decl = (tree)n->value;
> + 
> +   return true;
> + }
> + 
> + /* Merge the DECL_FUNCTION_INIT_TEST_TABLE from the function we're
> +    inlining.  */
> + 
> + void
> + java_inlining_merge_static_initializers (fn, decl_map)
> +      tree fn;
> +      void *decl_map;
> + {
> +   htab_traverse 
> +     (DECL_FUNCTION_INIT_TEST_TABLE (fn),
> +      merge_init_test_initialization, decl_map);
> + }
> + 
> + /* Lookup a DECL_FUNCTION_INIT_TEST_TABLE entry in the method we're
> +    inlining into.  If we already have a corresponding entry in that
> +    class we don't need to create another one, so we create a mapping
> +    from the variable in the inlined class to the corresponding
> +    pre-existing one.  */
> + 
> + static int
> + inline_init_test_initialization (entry, x)
> +      void * * entry;
> +      void * x;
> + {
> +   struct treetreehash_entry *ite = (struct treetreehash_entry *) *entry;
> +   splay_tree decl_map = (splay_tree)x;
> +   
> +   tree h = java_treetreehash_find 
> +     (DECL_FUNCTION_INIT_TEST_TABLE (current_function_decl), ite->key);
> +   if (! h)
> +     return true;
> + 
> +   splay_tree_insert (decl_map,
> + 		     (splay_tree_key) ite->value,
> + 		     (splay_tree_value) h);
> + 
> +   return true;
> + }
> + 
> + /* Look up the boolean variables in the DECL_FUNCTION_INIT_TEST_TABLE
> +    of a method being inlined.  For each hone, if we already have a
> +    variable associated with the same class in the method being inlined
> +    into, create a new mapping for it.  */
> + 
> + void
> + java_inlining_map_static_initializers (fn, decl_map)
> +      tree fn;
> +      void *decl_map;
> + {
> +   htab_traverse 
> +     (DECL_FUNCTION_INIT_TEST_TABLE (fn),
> +      inline_init_test_initialization, decl_map);
>   }
>   
> 

-- 
Why do so many programmers believe that they must meet some mythical
"complexity quota" each day?