* Simple returns are broken in gcc 3.X @ 2001-07-30 15:33 John David Anglin 2001-07-30 19:30 ` Richard Henderson 0 siblings, 1 reply; 14+ messages in thread From: John David Anglin @ 2001-07-30 15:33 UTC (permalink / raw) To: gcc In looking into why g++ didn't work for vax-dec-ultrix4.3, I have found that simple returns are broken. Compiling this small test program static char * srealloc (char *p, char *q, int i) { if (i == 0) return p; else return q; } yields #NO_APP .text .align 1 __Z8sreallocPcS_i: .word 0x0 subl2 $12,sp tstl 12(ap) jneq L2 ret L2: ret As can been seen, the return values are not loaded into the return register r0. The problem is that expand_value_return () doesn't handle properly return values in pseudos (see treatment in expand_function_end ()). Further, comparing what is done in expand_function_end and expand_null_return_1, there appears to be other problems. For example, instrumentation doesn't work. There could be problems with functtions that call alloca, etc. It looks to me like there needs to be a common set of code used to expand returns. Thoughts? Dave -- J. David Anglin dave.anglin@nrc.ca National Research Council of Canada (613) 990-0752 (FAX: 952-6605) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Simple returns are broken in gcc 3.X 2001-07-30 15:33 Simple returns are broken in gcc 3.X John David Anglin @ 2001-07-30 19:30 ` Richard Henderson 2001-07-30 20:38 ` John David Anglin 0 siblings, 1 reply; 14+ messages in thread From: Richard Henderson @ 2001-07-30 19:30 UTC (permalink / raw) To: John David Anglin; +Cc: gcc On Mon, Jul 30, 2001 at 06:33:14PM -0400, John David Anglin wrote: > It looks to me like there needs to be a common set of code used to expand > returns. Thoughts? You'd do just as well to disable the "return" pattern until after reload. Given that most other targets can't guarantee to use return directly until then you'll save yourself from executing code paths that no one else uses. Further, the post-reload optimizers can easily put the return everywhere it's needed, so you don't actually gain anything by generating it early. r~ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Simple returns are broken in gcc 3.X 2001-07-30 19:30 ` Richard Henderson @ 2001-07-30 20:38 ` John David Anglin 2001-07-30 23:16 ` Richard Henderson 0 siblings, 1 reply; 14+ messages in thread From: John David Anglin @ 2001-07-30 20:38 UTC (permalink / raw) To: Richard Henderson; +Cc: gcc > Further, the post-reload optimizers can easily put the return > everywhere it's needed, so you don't actually gain anything > by generating it early. Wouldn't it be better then to never output a simple return in expand_null_return_1? Something like: --- stmt.c.orig Thu May 17 07:37:30 2001 +++ stmt.c Mon Jul 30 23:27:50 2001 @@ -403,7 +403,7 @@ static void expand_nl_goto_receivers PARAMS ((struct nesting *)); static void fixup_gotos PARAMS ((struct nesting *, rtx, tree, rtx, int)); -static void expand_null_return_1 PARAMS ((rtx, int)); +static void expand_null_return_1 PARAMS ((rtx)); static void expand_value_return PARAMS ((rtx)); static int tail_recursion_args PARAMS ((tree, tree)); static void expand_cleanups PARAMS ((tree, tree, int, int)); @@ -2805,7 +2805,6 @@ void expand_null_return () { - struct nesting *block = block_stack; rtx last_insn = get_last_insn (); /* If this function was declared to return a value, but we @@ -2813,13 +2812,7 @@ propogated live to the rest of the function. */ clobber_return_register (); - /* Does any pending block have cleanups? */ - while (block && block->data.block.cleanups == 0) - block = block->next; - - /* If yes, use a goto to return, since that runs cleanups. */ - - expand_null_return_1 (last_insn, block != 0); + expand_null_return_1 (last_insn); } /* Generate RTL to return from the current function, with value VAL. */ @@ -2828,7 +2821,6 @@ expand_value_return (val) rtx val; { - struct nesting *block = block_stack; rtx last_insn = get_last_insn (); rtx return_reg = DECL_RTL (DECL_RESULT (current_function_decl)); @@ -2855,27 +2847,15 @@ emit_move_insn (return_reg, val); } - /* Does any pending block have cleanups? */ - - while (block && block->data.block.cleanups == 0) - block = block->next; - - /* If yes, use a goto to return, since that runs cleanups. - Use LAST_INSN to put cleanups *before* the move insn emitted above. */ - - expand_null_return_1 (last_insn, block != 0); + expand_null_return_1 (last_insn); } /* Output a return with no value. If LAST_INSN is nonzero, - pretend that the return takes place after LAST_INSN. - If USE_GOTO is nonzero then don't use a return instruction; - go to the return label instead. This causes any cleanups - of pending blocks to be executed normally. */ + pretend that the return takes place after LAST_INSN. */ static void -expand_null_return_1 (last_insn, use_goto) +expand_null_return_1 (last_insn) rtx last_insn; - int use_goto; { rtx end_label = cleanup_label ? cleanup_label : return_label; @@ -2883,27 +2863,8 @@ do_pending_stack_adjust (); last_expr_type = 0; - /* PCC-struct return always uses an epilogue. */ - if (current_function_returns_pcc_struct || use_goto) - { - if (end_label == 0) - end_label = return_label = gen_label_rtx (); - expand_goto_internal (NULL_TREE, end_label, last_insn); - return; - } - - /* Otherwise output a simple return-insn if one is available, - unless it won't do the job. */ -#ifdef HAVE_return - if (HAVE_return && use_goto == 0 && cleanup_label == 0) - { - emit_jump_insn (gen_return ()); - emit_barrier (); - return; - } -#endif - - /* Otherwise jump to the epilogue. */ + if (end_label == 0) + end_label = return_label = gen_label_rtx (); expand_goto_internal (NULL_TREE, end_label, last_insn); } \f Dave -- J. David Anglin dave.anglin@nrc.ca National Research Council of Canada (613) 990-0752 (FAX: 952-6605) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Simple returns are broken in gcc 3.X 2001-07-30 20:38 ` John David Anglin @ 2001-07-30 23:16 ` Richard Henderson 2001-07-31 8:03 ` John David Anglin 0 siblings, 1 reply; 14+ messages in thread From: Richard Henderson @ 2001-07-30 23:16 UTC (permalink / raw) To: John David Anglin; +Cc: gcc On Mon, Jul 30, 2001 at 11:35:45PM -0400, John David Anglin wrote: > Wouldn't it be better then to never output a simple return in > expand_null_return_1? Err, yes it would. There's another bit in expand_function_end. r~ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Simple returns are broken in gcc 3.X 2001-07-30 23:16 ` Richard Henderson @ 2001-07-31 8:03 ` John David Anglin 2001-07-31 9:56 ` Richard Henderson 0 siblings, 1 reply; 14+ messages in thread From: John David Anglin @ 2001-07-31 8:03 UTC (permalink / raw) To: Richard Henderson; +Cc: gcc > Err, yes it would. There's another bit in expand_function_end. Isn't that bit necessary if there isn't an epilogue? Dave -- J. David Anglin dave.anglin@nrc.ca National Research Council of Canada (613) 990-0752 (FAX: 952-6605) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Simple returns are broken in gcc 3.X 2001-07-31 8:03 ` John David Anglin @ 2001-07-31 9:56 ` Richard Henderson 2001-07-31 10:39 ` John David Anglin 0 siblings, 1 reply; 14+ messages in thread From: Richard Henderson @ 2001-07-31 9:56 UTC (permalink / raw) To: John David Anglin; +Cc: gcc On Tue, Jul 31, 2001 at 10:39:58AM -0400, John David Anglin wrote: > > Err, yes it would. There's another bit in expand_function_end. > > Isn't that bit necessary if there isn't an epilogue? No, look at thread_prologue_and_epilogue_insns. r~ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Simple returns are broken in gcc 3.X 2001-07-31 9:56 ` Richard Henderson @ 2001-07-31 10:39 ` John David Anglin 2001-07-31 13:13 ` Richard Henderson 0 siblings, 1 reply; 14+ messages in thread From: John David Anglin @ 2001-07-31 10:39 UTC (permalink / raw) To: Richard Henderson; +Cc: gcc > On Tue, Jul 31, 2001 at 10:39:58AM -0400, John David Anglin wrote: > > > Err, yes it would. There's another bit in expand_function_end. > > > > Isn't that bit necessary if there isn't an epilogue? > > No, look at thread_prologue_and_epilogue_insns. It looks like the "optimize" test needs to be removed from thread_prologue_and_epilogue_insns so that emit_return_into_block is always called. Is that OK? I presume that this is there because we didn't have a proper CFG when not optimizing. Dave -- J. David Anglin dave.anglin@nrc.ca National Research Council of Canada (613) 990-0752 (FAX: 952-6605) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Simple returns are broken in gcc 3.X 2001-07-31 10:39 ` John David Anglin @ 2001-07-31 13:13 ` Richard Henderson 2001-07-31 13:43 ` John David Anglin 0 siblings, 1 reply; 14+ messages in thread From: Richard Henderson @ 2001-07-31 13:13 UTC (permalink / raw) To: John David Anglin; +Cc: gcc On Tue, Jul 31, 2001 at 01:36:41PM -0400, John David Anglin wrote: > It looks like the "optimize" test needs to be removed from > thread_prologue_and_epilogue_insns so that emit_return_into_block > is always called. Is that OK? I presume that this is there > because we didn't have a proper CFG when not optimizing. Hmm. No, I suspect having a single return point -- and in particular no conditional returns -- is going to make debuggers happier with features like "xbreak". Grepping through sources, vax is the only target that does not define either HAVE_epilogue or TARGET_ASM_FUNCTION_EPILOGUE. I suggest that you add (define_expand "epilogue" [(return)] "" "") r~ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Simple returns are broken in gcc 3.X 2001-07-31 13:13 ` Richard Henderson @ 2001-07-31 13:43 ` John David Anglin 2001-07-31 13:49 ` Richard Henderson 0 siblings, 1 reply; 14+ messages in thread From: John David Anglin @ 2001-07-31 13:43 UTC (permalink / raw) To: Richard Henderson; +Cc: gcc > I suggest that you add > > (define_expand "epilogue" > [(return)] > "" > "") It may not be necessary. I am trying to rework thread_prologue_and_epilogue_insns to always output a return in the HAVE_return case. Dave -- J. David Anglin dave.anglin@nrc.ca National Research Council of Canada (613) 990-0752 (FAX: 952-6605) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Simple returns are broken in gcc 3.X 2001-07-31 13:43 ` John David Anglin @ 2001-07-31 13:49 ` Richard Henderson 2001-07-31 16:08 ` John David Anglin 0 siblings, 1 reply; 14+ messages in thread From: Richard Henderson @ 2001-07-31 13:49 UTC (permalink / raw) To: John David Anglin; +Cc: gcc On Tue, Jul 31, 2001 at 04:43:15PM -0400, John David Anglin wrote: > It may not be necessary. I am trying to rework > thread_prologue_and_epilogue_insns to always output a return > in the HAVE_return case. Hum. We'll see. I still think you'd be better served adding the epilogue pattern. It's clearer to categorize HAVE_return as the optimization and HAVE_epilogue as the requirement. r~ ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Simple returns are broken in gcc 3.X 2001-07-31 13:49 ` Richard Henderson @ 2001-07-31 16:08 ` John David Anglin 2001-07-31 16:37 ` Richard Henderson 0 siblings, 1 reply; 14+ messages in thread From: John David Anglin @ 2001-07-31 16:08 UTC (permalink / raw) To: Richard Henderson; +Cc: gcc > On Tue, Jul 31, 2001 at 04:43:15PM -0400, John David Anglin wrote: > > It may not be necessary. I am trying to rework > > thread_prologue_and_epilogue_insns to always output a return > > in the HAVE_return case. > > Hum. We'll see. I still think you'd be better served adding the > epilogue pattern. It's clearer to categorize HAVE_return as the > optimization and HAVE_epilogue as the requirement. I can't disagree with your logic. In looking at thread_prologue_and_epilogue_insns, I am having difficulty understanding how "returns" are generated when an exit block has no non-fake predecessors or no fall-thru predecessor. In these two situations, the code jumps to epilogue_done and the epilogue isn't expanded. Dave -- J. David Anglin dave.anglin@nrc.ca National Research Council of Canada (613) 990-0752 (FAX: 952-6605) ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Simple returns are broken in gcc 3.X 2001-07-31 16:08 ` John David Anglin @ 2001-07-31 16:37 ` Richard Henderson 0 siblings, 0 replies; 14+ messages in thread From: Richard Henderson @ 2001-07-31 16:37 UTC (permalink / raw) To: John David Anglin; +Cc: gcc On Tue, Jul 31, 2001 at 07:06:25PM -0400, John David Anglin wrote: > In looking at thread_prologue_and_epilogue_insns, I am having > difficulty understanding how "returns" are generated when an exit > block has no non-fake predecessors or no fall-thru predecessor. They aren't generated in this case because that means that the function doesn't return (normally) at all. Perhaps that bit is deserving of a comment... There are two other kinds of edges that can come to EXIT_BLOCK: (1) If EDGE_ABNORMAL_CALL, then we exit the function by a sibling call. (2) If !EDGE_FALLTHRU, then we exit via a return insn. Given that we're removing HAVE_return generation from early in the compiler, this should no longer happen at this point. r~ ^ permalink raw reply [flat|nested] 14+ messages in thread
[parent not found: <no.id>]
* Re: Simple returns are broken in gcc 3.X [not found] <no.id> @ 2001-08-09 15:12 ` John David Anglin 2001-08-09 15:48 ` Richard Henderson 0 siblings, 1 reply; 14+ messages in thread From: John David Anglin @ 2001-08-09 15:12 UTC (permalink / raw) To: John David Anglin; +Cc: gcc, gcc-patches > In looking into why g++ didn't work for vax-dec-ultrix4.3, I have found > that simple returns are broken. Compiling this small test program > > static char * > srealloc (char *p, char *q, int i) > { > if (i == 0) > return p; > else > return q; > } > > yields > > #NO_APP > .text > .align 1 > __Z8sreallocPcS_i: > .word 0x0 > subl2 $12,sp > tstl 12(ap) > jneq L2 > ret > L2: > ret This patch fixes the above problem by always forcing a "goto" return. The patch only affects the vax since it is the only remaining port that doesn't define an epilogue and allows the expansion of simple returns prior to reload. This patch changes the vax port to use the same code patch for return generation as all others. As a side benefit, the change also fixes function instrumentation on the vax. The above behavior is a regression versus previous gcc versions since g++ was functional on the vax in version 2.8.1. Full bootstrap has been done with vax-dec-ultrix4.3 with 3.0 branch. Bootstrap and regression checked with 3.0 branch on hppa1.1-hp-hpux10.20 and i686-pc-linux-gnu. Bootstrap and regression checked with 3.1 mainline on i686-pc-linux-gnu. OK for main? I would like it on the branch as well but there are a couple of other patches needed before C++ will be fully functional on the vax. Thus, it can wait until after 3.0.1 is released. Dave -- J. David Anglin dave.anglin@nrc.ca National Research Council of Canada (613) 990-0752 (FAX: 952-6605) 2001-08-09 John David Anglin <dave@hiauly1.hia.nrc.ca> * stmt.c (expand_null_return_1): Remove code to generate simple returns and "use_goto" argument. (expand_null_return, expand_value_return): Update all callers. * function.c (expand_function_end): Remove code to generate simple return. * config/vax/vax.md (epilogue): New expander for function return. * doc/md.texi (epilogue): Remove "if defined". --- stmt.c.orig Tue Jul 31 10:22:56 2001 +++ stmt.c Wed Aug 1 15:52:05 2001 @@ -403,7 +403,7 @@ static void expand_nl_goto_receivers PARAMS ((struct nesting *)); static void fixup_gotos PARAMS ((struct nesting *, rtx, tree, rtx, int)); -static void expand_null_return_1 PARAMS ((rtx, int)); +static void expand_null_return_1 PARAMS ((rtx)); static void expand_value_return PARAMS ((rtx)); static int tail_recursion_args PARAMS ((tree, tree)); static void expand_cleanups PARAMS ((tree, tree, int, int)); @@ -2863,7 +2863,6 @@ void expand_null_return () { - struct nesting *block = block_stack; rtx last_insn = get_last_insn (); /* If this function was declared to return a value, but we @@ -2871,13 +2870,7 @@ propogated live to the rest of the function. */ clobber_return_register (); - /* Does any pending block have cleanups? */ - while (block && block->data.block.cleanups == 0) - block = block->next; - - /* If yes, use a goto to return, since that runs cleanups. */ - - expand_null_return_1 (last_insn, block != 0); + expand_null_return_1 (last_insn); } /* Generate RTL to return from the current function, with value VAL. */ @@ -2886,7 +2879,6 @@ expand_value_return (val) rtx val; { - struct nesting *block = block_stack; rtx last_insn = get_last_insn (); rtx return_reg = DECL_RTL (DECL_RESULT (current_function_decl)); @@ -2913,27 +2905,15 @@ emit_move_insn (return_reg, val); } - /* Does any pending block have cleanups? */ - - while (block && block->data.block.cleanups == 0) - block = block->next; - - /* If yes, use a goto to return, since that runs cleanups. - Use LAST_INSN to put cleanups *before* the move insn emitted above. */ - - expand_null_return_1 (last_insn, block != 0); + expand_null_return_1 (last_insn); } /* Output a return with no value. If LAST_INSN is nonzero, - pretend that the return takes place after LAST_INSN. - If USE_GOTO is nonzero then don't use a return instruction; - go to the return label instead. This causes any cleanups - of pending blocks to be executed normally. */ + pretend that the return takes place after LAST_INSN. */ static void -expand_null_return_1 (last_insn, use_goto) +expand_null_return_1 (last_insn) rtx last_insn; - int use_goto; { rtx end_label = cleanup_label ? cleanup_label : return_label; @@ -2941,27 +2921,8 @@ do_pending_stack_adjust (); last_expr_type = 0; - /* PCC-struct return always uses an epilogue. */ - if (current_function_returns_pcc_struct || use_goto) - { - if (end_label == 0) - end_label = return_label = gen_label_rtx (); - expand_goto_internal (NULL_TREE, end_label, last_insn); - return; - } - - /* Otherwise output a simple return-insn if one is available, - unless it won't do the job. */ -#ifdef HAVE_return - if (HAVE_return && use_goto == 0 && cleanup_label == 0) - { - emit_jump_insn (gen_return ()); - emit_barrier (); - return; - } -#endif - - /* Otherwise jump to the epilogue. */ + if (end_label == 0) + end_label = return_label = gen_label_rtx (); expand_goto_internal (NULL_TREE, end_label, last_insn); } \f --- function.c.orig Sat Jun 9 15:22:26 2001 +++ function.c Wed Aug 1 12:40:23 2001 @@ -6949,18 +6949,6 @@ instead of using the general framework. */ use_return_register (); - /* Output a return insn if we are using one. - Otherwise, let the rtl chain end here, to drop through - into the epilogue. */ - -#ifdef HAVE_return - if (HAVE_return) - { - emit_jump_insn (gen_return ()); - emit_barrier (); - } -#endif - /* Fix up any gotos that jumped out to the outermost binding level of the function. Must follow emitting RETURN_LABEL. */ --- config/vax/vax.md.orig Sun Jan 14 04:08:51 2001 +++ config/vax/vax.md Tue Jul 31 22:08:45 2001 @@ -1926,6 +1926,15 @@ "" "ret") +(define_expand "epilogue" + [(return)] + "" + " +{ + emit_jump_insn (gen_return ()); + DONE; +}") + (define_insn "nop" [(const_int 0)] "" --- doc/md.texi.orig Thu Jun 14 18:54:21 2001 +++ doc/md.texi Thu Aug 9 12:24:04 2001 @@ -2859,7 +2859,7 @@ @cindex @code{epilogue} instruction pattern @item @samp{epilogue} -This pattern, if defined, emits RTL for exit from a function. The function +This pattern emits RTL for exit from a function. The function exit is responsible for deallocating the stack frame, restoring callee saved registers and emitting the return instruction. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: Simple returns are broken in gcc 3.X 2001-08-09 15:12 ` John David Anglin @ 2001-08-09 15:48 ` Richard Henderson 0 siblings, 0 replies; 14+ messages in thread From: Richard Henderson @ 2001-08-09 15:48 UTC (permalink / raw) To: John David Anglin; +Cc: gcc, gcc-patches On Thu, Aug 09, 2001 at 06:12:04PM -0400, John David Anglin wrote: > * stmt.c (expand_null_return_1): Remove code to generate simple returns > and "use_goto" argument. > (expand_null_return, expand_value_return): Update all callers. > * function.c (expand_function_end): Remove code to generate simple > return. > * config/vax/vax.md (epilogue): New expander for function return. > * doc/md.texi (epilogue): Remove "if defined". Ok. r~ ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2001-08-09 15:48 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2001-07-30 15:33 Simple returns are broken in gcc 3.X John David Anglin 2001-07-30 19:30 ` Richard Henderson 2001-07-30 20:38 ` John David Anglin 2001-07-30 23:16 ` Richard Henderson 2001-07-31 8:03 ` John David Anglin 2001-07-31 9:56 ` Richard Henderson 2001-07-31 10:39 ` John David Anglin 2001-07-31 13:13 ` Richard Henderson 2001-07-31 13:43 ` John David Anglin 2001-07-31 13:49 ` Richard Henderson 2001-07-31 16:08 ` John David Anglin 2001-07-31 16:37 ` Richard Henderson [not found] <no.id> 2001-08-09 15:12 ` John David Anglin 2001-08-09 15:48 ` Richard Henderson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).