This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [patch i386]: Sibcall tail-call improvement and partial fix PR/60104
- From: Iain Sandoe <iain at codesourcery dot com>
- To: Mike Stump <mikestump at comcast dot net>
- Cc: Kai Tietz <ktietz at redhat dot com>, Segher Boessenkool <segher at kernel dot crashing dot org>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Jeff Law <law at redhat dot com>, Richard Henderson <rth at redhat dot com>, FX <fxcoudert at gmail dot com>
- Date: Mon, 15 Sep 2014 11:57:40 +0100
- Subject: Re: [patch i386]: Sibcall tail-call improvement and partial fix PR/60104
- Authentication-results: sourceware.org; auth=none
- References: <1478265243 dot 5697739 dot 1400792508558 dot JavaMail dot zimbra at redhat dot com> <AD1FD69F-389F-47A9-83F5-7584696E2677 at comcast dot net> <20140915004309 dot GA29543 at gate dot crashing dot org> <95EF5F55-098D-49F3-B6E6-79C1316D5148 at comcast dot net>
Hi Mike,
On 15 Sep 2014, at 08:33, Mike Stump wrote:
> On Sep 14, 2014, at 5:43 PM, Segher Boessenkool <segher@kernel.crashing.org> wrote:
>> On Sun, Sep 14, 2014 at 02:38:45PM -0700, Mike Stump wrote:
>>> + SIBLING_CALL_P (tmp) = 1;
>>> + SIBLING_CALL_P (tmp) = 1;
>>
>> The second time is to make sure? :-)
>
> No, just a last minute cut and paste… I’ll remove it.
While the patch fixes the fallout from Kai's patch, I am concerned that:
1. It would be good to see how this [original] code path was tested on any other platform than Darwin (where it breaks).
- I.E. a non-Mach-O test case that exercises that path of the original patch's code.
- AFAICT this (exercise) does NOT happen for bootstrap and reg-test on x86-64-linux (so how was the original patch tested?).
2. The comment above the new code fragment has not been adjusted to reflect the new/changed functionality.
==
This has been ~ 3 months and the same questions / observations above have been raised on the PR thread and on @patches list. This does not seem to me to be a "darwin-only" issue, and just assuming that it's some unspecified fault with Darwin's address legalisation seems like an unwarranted leap (especially for x86-64, where Darwin shares a substantial part of its ABI with Linux).
Perhaps it would be safer simply to revert that hunk of the original patch unless/until (1) and (2) above are addressed?
0.02GBP, as usual,
Iain