From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 23888 invoked by alias); 17 Oct 2004 13:22:03 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 23880 invoked from network); 17 Oct 2004 13:22:02 -0000 Received: from unknown (HELO mail.libertysurf.net) (213.36.80.91) by sourceware.org with SMTP; 17 Oct 2004 13:22:02 -0000 Received: from dyn-213-36-36-80.ppp.tiscali.fr (213.36.36.80) by mail.libertysurf.net (6.5.036) id 417213A8000AF02B; Sun, 17 Oct 2004 15:22:01 +0200 From: Eric Botcazou To: gcc-patches@gcc.gnu.org Subject: Re: [PATCH] Fix PR middle-end/17813 Date: Sun, 17 Oct 2004 13:51:00 -0000 User-Agent: KMail/1.6.1 Cc: Roger Sayle References: In-Reply-To: MIME-Version: 1.0 Content-Disposition: inline Message-Id: <200410171519.34999.ebotcazou@libertysurf.fr> Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit X-SW-Source: 2004-10/txt/msg01371.txt.bz2 > This is OK for mainline. Thanks for the review. > A slight improvement might be to add a new ignore_pending_stack_adjust > function (like clear_pending_stack_adjust) that resets stack_pointer_delta > and pending_stack_adjust to zero, and call that in emit_stack_restore. I have 3 remarks/questions: - what about discard_pending_stack_adjust instead? I think that "ignore" could fool people into thinking that the stack adjustment is only temporarily suspended. - what about changing clear_pending_stack_adjust into maybe_discard_pending_stack_adjust? The rationale is that the typical idiom is currently: clear_pending_stack_adjust (); do_pending_stack_adjust (); which seems odd at best. - I think that stack_pointer_delta should not be reset to zero for strict correctness, and instead the same adjustment should be made in the new function as in clear_pending_stack_adjust. > Currently, we'll emit a "dead" stack adjustment immediately prior to > restoring the stack pointer, and then leave it to the RTL optimizers to > clean up. All we really need to do is flush the "memory" of the pending > adjustment. This was the acknowledged strategy used in the patch. Now, after Honza's remark, I think yours is better. -- Eric Botcazou