From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 7630 invoked by alias); 9 Oct 2007 15:42:57 -0000 Received: (qmail 7617 invoked by uid 22791); 9 Oct 2007 15:42:56 -0000 X-Spam-Check-By: sourceware.org Received: from smtp-out.google.com (HELO smtp-out.google.com) (216.239.33.17) by sourceware.org (qpsmtpd/0.31) with ESMTP; Tue, 09 Oct 2007 15:42:52 +0000 Received: from zps77.corp.google.com (zps77.corp.google.com [172.25.146.77]) by smtp-out.google.com with ESMTP id l99Fgbdw019283; Tue, 9 Oct 2007 16:42:38 +0100 Received: from localhost.localdomain.google.com (dhcp-172-18-216-111.corp.google.com [172.18.216.111]) (authenticated bits=0) by zps77.corp.google.com with ESMTP id l99Fgang005234 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NOT); Tue, 9 Oct 2007 08:42:37 -0700 To: "Uros Bizjak" Cc: "Kenneth Zadeck" , "GCC Patches" , "Eric Botcazou" Subject: Re: [PATCH, rtl-optimization]: Fix PR rtl-optimization/33638 References: <5787cf470710080514m2edaae59ma1f21c008de64800@mail.gmail.com> <200710090840.38213.ebotcazou@libertysurf.fr> <470B6B35.2090800@naturalbridge.com> <5787cf470710090531q14d11d16pbaca9fd8eb29b10f@mail.gmail.com> From: Ian Lance Taylor Date: Tue, 09 Oct 2007 15:42:00 -0000 In-Reply-To: <5787cf470710090531q14d11d16pbaca9fd8eb29b10f@mail.gmail.com> Message-ID: User-Agent: Gnus/5.09 (Gnus v5.9.0) Emacs/21.4 MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2007-10/txt/msg00483.txt.bz2 "Uros Bizjak" writes: > An (less intrusive?) variant of my previous patch is to change call to > memory_address in emit_push_insn() to memory_address_noforce(). I > can't see the benefit of > > load reg <- (SP + off) > store (reg) <- regX > > since IMO the address of the pushed argument won't be CSE'd with any > other address in any way. It can be CSE'd if you make two function calls in a row. Whether that CSE is useful is admittedly another question. > BTW: I have tried to update arg->stack just after the call to > emit_push_insn in store_one_arg with SET_DEST (PATTERN (get_last_insn > ())) to update CALL_INSN_FUNCTION_USAGE, but IMO, this is too hacky. But CALL_INSN_FUNCTION_USAGE has not been set at that point. The code calls store_one_arg, then adds an entry to CALL_INSN_FUNCTION_USAGE for a const function. You would just have to arrange for the call to store_one_arg to return the actual address that it uses. This is not a complete solution, as nothing prevents that address from getting munged in some other way between expand and dse. Arguably the bug is assuming that a note does a good job of conveying that an address is being used. This only works if the address doesn't change in any way, or if the other optimizations are smart enough to pick up (which in general they aren't). The note was added for PR 12372, which was about a similar case for a sibcall: the store was getting deleted before the jump. It wouldn't be surprising if the test case in PR 12372 failed with -fforce-addr when using flow. Ian