This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Fix unwinding through SA_ONSTACK signal frames on IA-64
- From: Gabriel Dos Reis <gdr at integrable-solutions dot net>
- To: Jim Wilson <wilson at specifixinc dot com>
- Cc: Jakub Jelinek <jakub at redhat dot com>, Richard Henderson <rth at redhat dot com>, gcc-patches at gcc dot gnu dot org, David Mosberger <davidm at hpl dot hp dot com>, Ulrich Drepper <drepper at redhat dot com>
- Date: 19 Dec 2003 11:08:56 +0100
- Subject: Re: [PATCH] Fix unwinding through SA_ONSTACK signal frames on IA-64
- Organization: Integrable Solutions
- References: <20031218150528.GB12344@sunsite.ms.mff.cuni.cz><1071819158.1058.107.camel@leaf.tuliptree.org>
Jim Wilson <wilson@specifixinc.com> writes:
| On Thu, 2003-12-18 at 07:05, Jakub Jelinek wrote:
| > Ok to commit? What about 3.3?
|
| 3.3 is Gaby's decision.
|
| This patch could be improved if some of the less obvious stuff had
| comments. For instance, you could explain why you replaced the
| builtin_ia64_flushrs call with an asm. It looks like you are clearing
| the mode and loadrs fields, which makes some sense. Maybe the builtin
| function needs to do that also? It isn't used anyplace else. Just
| below that, there is a place where you compute regstk_top by anding with
| a magic constant ~0x1ffull. It isn't obvious why you are doing that.
| This same magic constant is used in another place.
|
| The ia64_copy_rbs function should have an explanatory comment in front
| of it, as per GNU coding conventions. And it would help if there were
| some comments in the code; it is very hard to understand what it is
| doing. I didn't even bother to try. I will just have to accept your
| word on this.
|
| This looks OK to me with some comments added in.
With Jim's comment addressed, the patch is OK for 3.3 branch after 3
or 4 days on mainline.
| You didn't mention testing, I am assuming you did the usual regression
| testing.
-- Gaby