On Wed, Aug 24, 2011 at 9:46 AM, Bernd Schmidt <ber...@codesourcery.com> wrote:
> On 08/03/11 17:38, Richard Sandiford wrote:
>> Bernd Schmidt <ber...@codesourcery.com> writes:
>>> +@findex simple_return
>>> +@item (simple_return)
>>> +Like @code{(return)}, but truly represents only a function return, while
>>> +@code{(return)} may represent an insn that also performs other functions
>>> +of the function epilogue. Like @code{(return)}, this may also occur in
>>> +conditional jumps.
>>
>> Sorry, I've forgotton the outcome of the discussion about what happens
>> on targets whose return expands to the same code as their simple_return.
>> Do the targets still need both "return" and "simple_return" rtxes?
>
> It's important to distinguish between these names as rtxes that can
> occur in instruction patterns, and a use as a standard pattern name.
> When a "return" pattern is generated, it should either fail or expand to
> something that performs both the epilogue and the return. A
> "simple_return" expands to something that performs only the return.
>
> Most targets allow "return" patterns only if the epilogue is empty. In
> that case, "return" and "simple_return" can expand to the same insn; it
> does not matter if that insn uses "simple_return" or "return", as they
> are equivalent in the absence of an epilogue. It would be slightly nicer
> to use "simple_return" in the patterns everywhere except ARM, but ports
> don't need to be changed.
>
>> Do they need both md patterns (but potentially using the same rtx
>> underneath)?
>
> The "return" standard pattern is needed for the existing optimizations
> (inserting returns in-line rather than jumping to the end of the
> function). Typically, it always fails if the function needs an epilogue,
> except in the ARM case.
> For shrink-wrapping to work, a port needs a "simple_return" pattern,
> which the compiler can use even if parts of the function need an
> epilogue. So yes, they have different purposes.
>
>> I ask because the rtl.def comment implies that those targets still
>> need both expanders and both rtxes. If that's so, I think it needs
>> to be mentioned here too. E.g. something like:
>>
>> Like @code{(return)}, but truly represents only a function return, while
>> @code{(return)} may represent an insn that also performs other functions
>> of the function epilogue. @code{(return)} only occurs on paths that
>> pass through the function prologue, while @code{(simple_return)}
>> only occurs on paths that do not pass through the prologue.
>
> This is not accurate for the rtx code. It is mostly accurate for the
> standard pattern name. A simple_return rtx may occur just after an
> epilogue, i.e. on a path that has passed through the prologue.
>
> Even for the simple_return pattern, I'm not sure reorg.c couldn't
> introduce new expansions in a location after both prologue and epilogue.
>
>> Like @code{(return)}, @code{(simple_return)} may also occur in
>> conditional jumps.
>>
>> You need to document the simple_return pattern in md.texi too.
>
> I was trying to update the documentation to only the current state after
> the patch. The thinking was that without shrink-wrapping, nothing
> generates this pattern, so documenting it would be misleading.
> However, with the mips changes in this version of the patch, reorg.c
> does make use of this pattern, so I've added documentation
>
>>> @@ -3498,6 +3506,8 @@ relax_delay_slots (rtx first)
>>> continue;
>>>
>>> target_label = JUMP_LABEL (delay_insn);
>>> + if (target_label && ANY_RETURN_P (target_label))
>>> + continue;
>>>
>>> if (!ANY_RETURN_P (target_label))
>>> {
>>
>> This doesn't look like a pure "handle return as well as simple return"
>> change. Is the idea that every following test only makes sense for
>> labels, and that things like:
>>
>> && prev_active_insn (target_label) == insn
>>
>> (to pick just one example) are actively dangerous for returns?
>
> That probably was the idea. Looking at it again, there's one case at the
> bottom of the loop which may be safe, but given that there were no code
> generation differences with the patch on three targets with
> define_delay, I've done:
>
>> If so, I think you should remove the immediately-following.
>> "if (!ANY_RETURN_P (target_label))" condition and reindent the body.
>
> this.
>
>> Given what you said about JUMP_LABEL sometimes being null,
>> I think we need either (a) to check whether each *_return_label
>> is null before comparing it with JUMP_LABEL, or (b) to ensure that
>> we're dealing with a jump to a label. (b) seems neater IMO
>> (as a call to jump_to_label_p).
>
> Done.
>
>>
>>> +#if defined HAVE_return || defined HAVE_simple_return
>>> + if (
>>> #ifdef HAVE_return
>>> - if (HAVE_return && end_of_function_label != 0)
>>> + (HAVE_return && function_return_label != 0)
>>> +#else
>>> + 0
>>> +#endif
>>> +#ifdef HAVE_simple_return
>>> + || (HAVE_simple_return && function_simple_return_label != 0)
>>> +#endif
>>> + )
>>> make_return_insns (first);
>>> #endif
>>
>> Eww.
>
> Restructured.
>
>> (define_code_iterator any_return [return simple_return])
>>
>> and just change the appropriate returns to any_returns.
>
> I've done this a bit differently - to show that it can be done, I've
> changed mips to always emit simple_return rtxs, even for "return"
> patterns (no code generation changes observed again).
>
> This version regression tested on mips64-elf, c/c++/objc.
>
>
> Bernd
>
This caused:
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50215
--
H.J.
