could someone try this patch?

[Ranjit Mathew]

Per Bothner wrote:
> Could someone test this patch?  I'm interested not only in whether it is
> correct (i.e. no regressions) but also how much of a speed-up, if any,
> if causes when compiling from source.  (So timing "make bootstrap"
> and "make checking", with --disable-checking, before and after, would
> be a good data point.)

I tried this patch with current mainline configured with
"--disable-checking" and "--enable-languages=c,c++,java":

$ $MYGCJ -v
Reading specs from /home/ranmath/src/gcc/build/gcc/specs
Reading specs from /home/ranmath/src/gcc/build/i686-pc-linux-gnu/libjava/libgcj.
spec
rename spec lib to liborig
Target: i686-pc-linux-gnu
Configured with: /home/ranmath/src/gcc/gcc-20050426/configure --prefix=/home/ran
math/gcc --enable-languages=c,c++,java --disable-nls --disable-checking --disabl
e-libmudflap --disable-debug --enable-threads=posix --enable-__cxa_atexit
Thread model: posix
gcc version 4.1.0 20050426 (experimental)

Here are the results.

"time make bootstrap" - before the patch:
  68m 13.522s (54m 4.560s user, 13m 15.640s system)

"time make bootstrap" - after the patch:
  68m 7.530s (53m 55.650s user, 13m 16.030s system)

I did regression testing with Jacks included. There
were three additional failures in Jacks (and none
elsewhere):

  FAIL: 9.2-implicit-11
  FAIL: 9.2-implicit-12
  FAIL: 9.2-implicit-17

Post-patch GCJ fails to complain about these testcases:
----------------------------- 8< -----------------------------
$ cat T92i11.java

class T92i11 {

        interface I { int clone(); }
        abstract class C implements I {}

}
$ cat T92i12.java

class T92i12 {

        interface I { int finalize(); }
        abstract class C implements I {}

}
$ cat T92i17.java

class T92i17 {

        interface I { Object clone() throws java.io.IOException; }
        abstract class C implements I {}

}
$
----------------------------- 8< -----------------------------

To be fair though, the pre-patch GCJ used to ICE on all of
these testcases and thus "pass" them inadvertently:
----------------------------- 8< -----------------------------
$ $MYGCJ -C T92i11.java
T92i11.java:2: internal compiler error: Segmentation fault
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.

$ $MYGCJ -C T92i12.java
T92i12.java:2: internal compiler error: Segmentation fault
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.

$ $MYGCJ -C T92i17.java
T92i17.java:2: internal compiler error: Segmentation fault
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://gcc.gnu.org/bugs.html> for instructions.
$
----------------------------- 8< -----------------------------

:-/

Thanks,
Ranjit.


> I'm about to drive down to Southern California for a vacation where
> I should have email access, but leaving the computer running all
> day for a bootstrap+check might be difficult.
> 
> The patch is partly cleanup (with I'll check in separately) and partly
> fixing (I hope) a performance bug: When compiling class A which
> requires B.java which is *not* on the compand line, and B.java
> has a local variable of type C, then it seems silly to try to
> read C - we don't *care* about the internals of C's methods.
> 
> The exception is if we might try to inline the method in B.java,
> in which case the patch is probably incorrect.  I'm not sure if
> that can be an issue at -O2 - if so, the patch probably might not
> allow bootstrapping.
> 
> 
> ------------------------------------------------------------------------
> 
> ? PATCHES
> ? jv-misc.patch
> ? patch1
> Index: decl.c
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/java/decl.c,v
> retrieving revision 1.218
> diff -u -p -r1.218 decl.c
> --- decl.c	23 Apr 2005 21:29:01 -0000	1.218
> +++ decl.c	25 Apr 2005 15:40:31 -0000
> @@ -504,6 +504,10 @@ static tree shadowed_labels;
>  #endif
>  
>  tree java_global_trees[JTI_MAX];
> +
> +/* See java-tree.h. */
> +tree main_class;
> +tree current_class;
>    
>  /* Build (and pushdecl) a "promoted type" for all standard
>     types shorter than int.  */
> Index: java-tree.h
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/java/java-tree.h,v
> retrieving revision 1.229
> diff -u -p -r1.229 java-tree.h
> --- java-tree.h	8 Apr 2005 01:02:59 -0000	1.229
> +++ java-tree.h	25 Apr 2005 15:40:31 -0000
> @@ -79,7 +79,7 @@ struct JCF;
>     2: CLASS_PARSED_P (in RECORD_TYPE).
>     3: CLASS_FROM_SOURCE_P (in RECORD_TYPE).
>     4: CLASS_P (in RECORD_TYPE).
> -   5: CLASS_FROM_CURRENTLY_COMPILED_SOURCE_P (in RECORD_TYPE)
> +   5: CLASS_FROM_CURRENTLY_COMPILED_P (in RECORD_TYPE)
>     6: CLASS_BEING_LAIDOUT (in RECORD_TYPE)
>  
>     Usage of DECL_LANG_FLAG_?:
> @@ -133,15 +133,13 @@ struct JCF;
>  extern int compiling_from_source;
>  
>  /* The class defined by the actual (main) file we are compiling. */
> -#define main_class \
> -  java_global_trees[JTI_MAIN_CLASS]
> +extern GTY(()) tree main_class;
>  
>  /* The class we use as the base for name resolution.  It's usually the
>     class we're generating code for but sometimes it points to an inner
>     class.  If you really want to know the class we're currently
>     generating code for, use output_class instead.  */
> -#define current_class \
> -  java_global_trees[JTI_CURRENT_CLASS]
> +extern GTY(()) tree current_class;
>  
>  /* The class we are currently generating.  Really.  */
>  #define output_class \
> @@ -406,8 +404,6 @@ enum java_tree_index
>  
>    JTI_WFL_OPERATOR,
>  
> -  JTI_MAIN_CLASS,
> -  JTI_CURRENT_CLASS,
>    JTI_OUTPUT_CLASS,
>    JTI_ALL_CLASS_LIST,
>  
> Index: parse.y
> ===================================================================
> RCS file: /cvs/gcc/gcc/gcc/java/parse.y,v
> retrieving revision 1.532
> diff -u -p -r1.532 parse.y
> --- parse.y	23 Apr 2005 21:29:04 -0000	1.532
> +++ parse.y	25 Apr 2005 15:40:35 -0000
> @@ -242,7 +242,7 @@ static int check_thrown_exceptions_do (t
>  static void purge_unchecked_exceptions (tree);
>  static bool ctors_unchecked_throws_clause_p (tree);
>  static void check_concrete_throws_clauses (tree, tree, tree, tree);
> -static void check_throws_clauses (tree, tree, tree);
> +static void check_throws_clauses (tree, tree);
>  static void finish_method_declaration (tree);
>  static tree build_super_invocation (tree);
>  static int verify_constructor_circularity (tree, tree);
> @@ -4170,6 +4170,7 @@ create_class (int flags, tree id, tree s
>  		  ctxp->interface_number);
>    ctxp->interface_number = 0;
>    CLASS_COMPLETE_P (decl) = 1;
> +  current_class = TREE_TYPE (decl);
>    add_superinterfaces (decl, interfaces);
>  
>    /* TYPE_VFIELD' is a compiler-generated field used to point to
> @@ -4211,6 +4212,8 @@ end_class_declaration (int resume)
>    POP_CPC ();
>    if (resume && no_error_occurred)
>      java_parser_context_resume ();
> +  else
> +    current_class = NULL_TREE;
>  
>    /* We're ending a class declaration, this is a good time to reset
>       the interface cout. Note that might have been already done in
> @@ -6663,8 +6666,7 @@ check_interface_throws_clauses (tree che
>  	      if (method != NULL_TREE && !METHOD_ABSTRACT (method)
>  		  && !METHOD_INVISIBLE (iface_method))
>  		{
> -		  tree method_wfl = DECL_FUNCTION_WFL (method);
> -		  check_throws_clauses (method, method_wfl, iface_method);
> +		  check_throws_clauses (method, iface_method);
>  		}
>  	    }
>  
> @@ -6681,27 +6683,30 @@ static void
>  check_concrete_throws_clauses (tree class, tree self_method,
>  			       tree name, tree signature)
>  {
> -  tree method = lookup_argument_method_generic (class, name, signature,
> -						SEARCH_SUPER | SEARCH_VISIBLE);
> -  while (method != NULL_TREE)
> +  tree context = class;
> +  for (;;)
>      {
> +      tree method = lookup_argument_method_generic (context, name, signature,
> +						    SEARCH_SUPER | SEARCH_VISIBLE);
> +      if (method == NULL_TREE)
> +	break;
>        if (! METHOD_INVISIBLE (method) && hack_is_accessible_p (method, class))
> -	check_throws_clauses (self_method, DECL_FUNCTION_WFL (self_method),
> -			      method);
> -
> -      method = lookup_argument_method_generic (DECL_CONTEXT (method),
> -					       name, signature,
> -					       SEARCH_SUPER | SEARCH_VISIBLE);
> +	check_throws_clauses (self_method, method);
> +      context = DECL_CONTEXT (method);
>      }
>  }
>  
>  /* Generate an error if the `throws' clause of METHOD (if any) is
>     incompatible with the `throws' clause of FOUND (if any).  */
>  static void
> -check_throws_clauses (tree method, tree method_wfl, tree found)
> +check_throws_clauses (tree method, tree found)
>  {
>    tree mthrows;
> -
> +  tree method_wfl =  DECL_FUNCTION_WFL (method);
> +  tree class = DECL_CONTEXT (method);
> +  if (! CLASS_FROM_CURRENTLY_COMPILED_P (class)
> +      || ! CLASS_FROM_SOURCE_P (class))
> +    return;
>    for (mthrows = DECL_FUNCTION_THROWS (method);
>         mthrows; mthrows = TREE_CHAIN (mthrows))
>      {
> @@ -7419,6 +7424,9 @@ declare_local_variables (int modifier, t
>        BLOCK_IS_IMPLICIT (b) = 1;
>      }
>  
> +  if (! CLASS_FROM_CURRENTLY_COMPILED_P (current_class))
> +    return;
> +
>    if (modifier)
>      {
>        size_t i;


-- 
Ranjit Mathew      Email: rmathew AT gmail DOT com

Bangalore, INDIA.    Web: http://ranjitmathew.hostingzero.com/