try/finally improvement

try/finally improvement

[Richard Henderson]

Changes try/finally to only use the control-flow-destroying
GOTO_SUBROUTINE_EXPR when there is no other alternative.  We
much prefer to simply re-expand the finally block in all the
places it must be invoked.

In the process, this uncovered a latent Java bug in that it
re-uses BLOCK in a way that the other front ends don't, and
so cannot re-expand BLOCK meaningfully.

This doesn't help libjava all that much, simply because the
construct isn't used that often.  Nevertheless...

Tested on x86 linux.


r~


        * expr.c (expand_expr) [TRY_FINALLY_EXPR]: Don't use
        GOTO_SUBROUTINE_EXPR when finally_block can be re-expanded.

        * lang.c (java_unsafe_for_reeval): New.
        (LANG_HOOKS_UNSAFE_FOR_REEVAL): New.

Index: gcc/expr.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/expr.c,v
retrieving revision 1.469
diff -c -p -d -r1.469 expr.c
*** gcc/expr.c	16 Jul 2002 10:59:15 -0000	1.469
--- gcc/expr.c	22 Jul 2002 00:40:04 -0000
*************** expand_expr (exp, target, tmode, modifie
*** 8965,8993 ****
        {
  	tree try_block = TREE_OPERAND (exp, 0);
  	tree finally_block = TREE_OPERAND (exp, 1);
- 	rtx finally_label = gen_label_rtx ();
- 	rtx done_label = gen_label_rtx ();
- 	rtx return_link = gen_reg_rtx (Pmode);
- 	tree cleanup = build (GOTO_SUBROUTINE_EXPR, void_type_node,
- 			      (tree) finally_label, (tree) return_link);
- 	TREE_SIDE_EFFECTS (cleanup) = 1;
  
! 	/* Start a new binding layer that will keep track of all cleanup
! 	   actions to be performed.  */
! 	expand_start_bindings (2);
  
! 	target_temp_slot_level = temp_slot_level;
  
! 	expand_decl_cleanup (NULL_TREE, cleanup);
! 	op0 = expand_expr (try_block, target, tmode, modifier);
  
- 	preserve_temp_slots (op0);
- 	expand_end_bindings (NULL_TREE, 0, 0);
- 	emit_jump (done_label);
- 	emit_label (finally_label);
- 	expand_expr (finally_block, const0_rtx, VOIDmode, 0);
- 	emit_indirect_jump (return_link);
- 	emit_label (done_label);
  	return op0;
        }
  
--- 8965,9013 ----
        {
  	tree try_block = TREE_OPERAND (exp, 0);
  	tree finally_block = TREE_OPERAND (exp, 1);
  
!         if (unsafe_for_reeval (finally_block) > 1)
! 	  {
! 	    /* In this case, wrapping FINALLY_BLOCK in an UNSAVE_EXPR
! 	       is not sufficient, so we cannot expand the block twice.
! 	       So we play games with GOTO_SUBROUTINE_EXPR to let us
! 	       expand the thing only once.  */
  
! 	    rtx finally_label = gen_label_rtx ();
! 	    rtx done_label = gen_label_rtx ();
! 	    rtx return_link = gen_reg_rtx (Pmode);
! 	    tree cleanup = build (GOTO_SUBROUTINE_EXPR, void_type_node,
! 			          (tree) finally_label, (tree) return_link);
! 	    TREE_SIDE_EFFECTS (cleanup) = 1;
  
! 	    /* Start a new binding layer that will keep track of all cleanup
! 	       actions to be performed.  */
! 	    expand_start_bindings (2);
! 	    target_temp_slot_level = temp_slot_level;
! 
! 	    expand_decl_cleanup (NULL_TREE, cleanup);
! 	    op0 = expand_expr (try_block, target, tmode, modifier);
! 
! 	    preserve_temp_slots (op0);
! 	    expand_end_bindings (NULL_TREE, 0, 0);
! 	    emit_jump (done_label);
! 	    emit_label (finally_label);
! 	    expand_expr (finally_block, const0_rtx, VOIDmode, 0);
! 	    emit_indirect_jump (return_link);
! 	    emit_label (done_label);
! 	  }
! 	else
! 	  {
! 	    expand_start_bindings (2);
! 	    target_temp_slot_level = temp_slot_level;
! 
! 	    expand_decl_cleanup (NULL_TREE, finally_block);
! 	    op0 = expand_expr (try_block, target, tmode, modifier);
! 
! 	    preserve_temp_slots (op0);
! 	    expand_end_bindings (NULL_TREE, 0, 0);
! 	  }
  
  	return op0;
        }
  
Index: gcc/java/lang.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/java/lang.c,v
retrieving revision 1.104
diff -c -p -d -r1.104 lang.c
*** gcc/java/lang.c	7 Jul 2002 22:10:18 -0000	1.104
--- gcc/java/lang.c	22 Jul 2002 00:40:04 -0000
*************** static void java_print_error_function PA
*** 61,66 ****
--- 61,67 ----
  static int process_option_with_no PARAMS ((const char *,
  					   const struct string_option *,
  					   int));
+ static int java_unsafe_for_reeval PARAMS ((tree));
  
  #ifndef TARGET_OBJECT_SUFFIX
  # define TARGET_OBJECT_SUFFIX ".o"
*************** struct language_function GTY(())
*** 238,243 ****
--- 239,246 ----
  #define LANG_HOOKS_POST_OPTIONS java_post_options
  #undef LANG_HOOKS_PARSE_FILE
  #define LANG_HOOKS_PARSE_FILE java_parse_file
+ #undef LANG_HOOKS_UNSAFE_FOR_REEVAL
+ #define LANG_HOOKS_UNSAFE_FOR_REEVAL java_unsafe_for_reeval
  #undef LANG_HOOKS_MARK_ADDRESSABLE
  #define LANG_HOOKS_MARK_ADDRESSABLE java_mark_addressable
  #undef LANG_HOOKS_EXPAND_EXPR
*************** java_post_options ()
*** 792,797 ****
--- 795,820 ----
  
    /* Initialize the compiler back end.  */
    return false;
+ }
+ 
+ /* Called from unsafe_for_reeval.  */
+ static int
+ java_unsafe_for_reeval (t)
+      tree t;
+ {
+   switch (TREE_CODE (t))
+     {
+     case BLOCK:
+       /* Our expander tries to expand the variables twice.  Boom.  */
+       if (BLOCK_EXPR_DECLS (t) != NULL)
+ 	return 2;
+       return unsafe_for_reeval (BLOCK_EXPR_BODY (t));
+ 
+     default:
+       break;
+     }
+ 
+   return -1;
  }
  
  #include "gt-java-lang.h"

Re: try/finally improvement

[Jeff Sturm]

On Sun, 21 Jul 2002, Richard Henderson wrote:
> This doesn't help libjava all that much, simply because the
> construct isn't used that often.  Nevertheless...

It should help every synchronized block.  There are many of these in
libgcj.

I'd expect your patch to shrink libgcj noticeably.

Jeff

Re: try/finally improvement

[Per Bothner]

Richard Henderson wrote:
> Changes try/finally to only use the control-flow-destroying
> GOTO_SUBROUTINE_EXPR when there is no other alternative.  We
> much prefer to simply re-expand the finally block in all the
> places it must be invoked.

The finally block could be a substantial block of code.  So
I assume that re-expanding the finally block is not done when
optimizing for space?

Also, re-expanding the finally block complicates setting breakpoints,
so you probably don't want to do it when compiling with -O0 or -O1.
In principle the descision to re-expand the finally block is
similar to whether to whether to inline functions, and so should
depend on various factors, including the size of the inlined
function (i.e. finally block).

(You've probably thought about these issues; I'm just making sure.)
-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/

Re: try/finally improvement

[Per Bothner]

Jeff Sturm wrote:
> It should help every synchronized block.  There are many of these in
> libgcj.

Regarless of what I wrote in my previous message. in the case of
TRY_FINALLY_EXPR used to implement synchronized blocks it should
be ok to always re-expand, even with -O0 and -O1.  If optimizing
for space it might be better to not re-expand, but it should be close.
-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/

Re: try/finally improvement

[Richard Henderson]

On Tue, Jul 23, 2002 at 11:17:58AM -0700, Per Bothner wrote:
> The finally block could be a substantial block of code.  So
> I assume that re-expanding the finally block is not done when
> optimizing for space?

No, no difference is done wrt -Os.  When I looked through libjava,
I didn't see any finally block that didn't consist of a single
function call or less, so it didn't seem worth it.

> Also, re-expanding the finally block complicates setting breakpoints,
> so you probably don't want to do it when compiling with -O0 or -O1.

Perhaps.  Ideally the debugger would set multiple "real"
breakpoints in response to the single command.

> In principle the descision to re-expand the finally block is
> similar to whether to whether to inline functions, and so should
> depend on various factors, including the size of the inlined
> function (i.e. finally block).

In principal, yes.  Though that decision is affected by the
horribleness that computed gotos have on the control flow.
If we supported a real gosub/return mechanism in the optimizers
it might be different.

> (You've probably thought about these issues; I'm just making sure.)

Thought about them.  Didn't come up with any real solutions.


r~

Re: try/finally improvement

[Per Bothner]

Richard Henderson wrote:
> No, no difference is done wrt -Os.  When I looked through libjava,
> I didn't see any finally block that didn't consist of a single
> function call or less, so it didn't seem worth it.

While most finally-s are pretty simple, there may be some that restore
some non-trivial state.  Given that you anyway have to handle the case
that you cannot expand the block twice, it seems reasonable to add a
test for -Os.

>>Also, re-expanding the finally block complicates setting breakpoints,
>>so you probably don't want to do it when compiling with -O0 or -O1.
> 
> Perhaps.  Ideally the debugger would set multiple "real"
> breakpoints in response to the single command.

When gdb can do that and do it without confusing the user,
then we can consider expanding the code multiple times when
compiling with -Oo or -O1.  Until then, I say let's not.

> In principal, yes.  Though that decision is affected by the
> horribleness that computed gotos have on the control flow.

Substantially worse than indirect unknown function calls and
virtual method calls?  If not, correct debugger semantics
outweighs the cost (until you get to -O2).
-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/

Re: try/finally improvement

[Richard Henderson]

On Tue, Jul 23, 2002 at 06:53:32PM -0700, Per Bothner wrote:
> > In principal, yes.  Though that decision is affected by the
> > horribleness that computed gotos have on the control flow.
> 
> Substantially worse than indirect unknown function calls and
> virtual method calls?

Yes.  Much.  Indirect branches are the very worst thing
you can do to our optimizers.


r~

Re: try/finally improvement

[Per Bothner]

Richard Henderson wrote:
> Yes.  Much.  Indirect branches are the very worst thing
> you can do to our optimizers.

I still think correct debugging is more important at -O0,
and probably also at -O1.
-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/

Re: try/finally improvement

[Richard Henderson]

On Tue, Jul 23, 2002 at 10:04:13PM -0700, Per Bothner wrote:
> I still think correct debugging is more important at -O0,
> and probably also at -O1.

I don't necessarily agree with -O1.  How about this?


r~


        * expr.c (expand_expr) [TRY_FINALLY_EXPR]: Use GOTO_SUBROUTINE_EXPR
        form when not optimizing.

Index: expr.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/expr.c,v
retrieving revision 1.470
diff -c -p -d -r1.470 expr.c
*** expr.c	22 Jul 2002 00:42:52 -0000	1.470
--- expr.c	24 Jul 2002 22:01:28 -0000
*************** expand_expr (exp, target, tmode, modifie
*** 8966,8977 ****
  	tree try_block = TREE_OPERAND (exp, 0);
  	tree finally_block = TREE_OPERAND (exp, 1);
  
!         if (unsafe_for_reeval (finally_block) > 1)
  	  {
  	    /* In this case, wrapping FINALLY_BLOCK in an UNSAVE_EXPR
  	       is not sufficient, so we cannot expand the block twice.
  	       So we play games with GOTO_SUBROUTINE_EXPR to let us
  	       expand the thing only once.  */
  
  	    rtx finally_label = gen_label_rtx ();
  	    rtx done_label = gen_label_rtx ();
--- 8966,8983 ----
  	tree try_block = TREE_OPERAND (exp, 0);
  	tree finally_block = TREE_OPERAND (exp, 1);
  
!         if (!optimize || unsafe_for_reeval (finally_block) > 1)
  	  {
  	    /* In this case, wrapping FINALLY_BLOCK in an UNSAVE_EXPR
  	       is not sufficient, so we cannot expand the block twice.
  	       So we play games with GOTO_SUBROUTINE_EXPR to let us
  	       expand the thing only once.  */
+ 	    /* When not optimizing, we go ahead with this form since
+ 	       (1) user breakpoints operate more predictably without
+ 		   code duplication, and
+ 	       (2) we're not running any of the global optimizers
+ 	           that would explode in time/space with the highly
+ 		   connected CFG created by the indirect branching.  */
  
  	    rtx finally_label = gen_label_rtx ();
  	    rtx done_label = gen_label_rtx ();

Re: try/finally improvement

[Per Bothner]

Richard Henderson wrote:
> On Tue, Jul 23, 2002 at 10:04:13PM -0700, Per Bothner wrote:
> 
>>I still think correct debugging is more important at -O0,
>>and probably also at -O1.
> 
> I don't necessarily agree with -O1.  How about this?

I'm not sure exactly what our standard is for
debuggability at -O1.  But I guess rthis is reasonable.

I'd add in a ... || optimize_size || ...
-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/

Re: try/finally improvement

[Richard Henderson]

On Thu, Jul 25, 2002 at 08:12:37AM -0700, Per Bothner wrote:
> I'd add in a ... || optimize_size || ...

I'd rather not, for the reason expressed in the comment: -Os runs
global optimizers, which *really* hate highly connected graphs.


r~

Re: try/finally improvement

[Per Bothner]

Richard Henderson wrote:
> On Thu, Jul 25, 2002 at 08:12:37AM -0700, Per Bothner wrote:
> 
>>I'd add in a ... || optimize_size || ...
> 
> I'd rather not, for the reason expressed in the comment: -Os runs
> global optimizers, which *really* hate highly connected graphs.

I won't object further.
-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/per/