From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 790 invoked by alias); 2 Jun 2011 05:00:13 -0000 Received: (qmail 765 invoked by uid 22791); 2 Jun 2011 05:00:12 -0000 X-SWARE-Spam-Status: No, hits=-1.7 required=5.0 tests=AWL,BAYES_00,TW_FC,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mail.codesourcery.com (HELO mail.codesourcery.com) (38.113.113.100) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 02 Jun 2011 04:59:54 +0000 Received: (qmail 9851 invoked from network); 2 Jun 2011 04:59:53 -0000 Received: from unknown (HELO ?192.168.1.16?) (cltang@127.0.0.2) by mail.codesourcery.com with ESMTPA; 2 Jun 2011 04:59:53 -0000 Message-ID: <4DE718CE.3040609@codesourcery.com> Date: Thu, 02 Jun 2011 05:00:00 -0000 From: Chung-Lin Tang User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.2.17) Gecko/20110414 Lightning/1.0b2 Thunderbird/3.1.10 MIME-Version: 1.0 To: Ramana Radhakrishnan CC: gcc-patches , Richard Earnshaw , Ramana Radhakrishnan , rdsandiford@googlemail.com Subject: Ping Re: [patch, ARM] Fix PR42017, LR not used in leaf functions References: <4DB8DB6D.1050107@codesourcery.com> <4DD273C4.6020104@codesourcery.com> <4DD65363.5010102@linaro.org> <4DD654AE.7070301@codesourcery.com> <4DDD3C90.4020000@codesourcery.com> In-Reply-To: <4DDD3C90.4020000@codesourcery.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: 2011-06/txt/msg00122.txt.bz2 Ping. On 2011/5/26 01:29 AM, Chung-Lin Tang wrote: > On 2011/5/20 07:46 PM, Chung-Lin Tang wrote: >> On 2011/5/20 下午 07:41, Ramana Radhakrishnan wrote: >>> On 17/05/11 14:10, Chung-Lin Tang wrote: >>>> On 2011/5/13 04:26 PM, Richard Sandiford wrote: >>>>> Richard Sandiford writes: >>>>>> Chung-Lin Tang writes: >>>>>>> My fix here simply adds 'reload_completed' as an additional condition >>>>>>> for EPILOGUE_USES to return true for LR_REGNUM. I think this should be >>>>>>> valid, as correct LR save/restoring is handled by the >>>>>>> epilogue/prologue >>>>>>> code; it should be safe for IRA to treat it as a normal call-used >>>>>>> register. >>>>>> >>>>>> FWIW, epilogue_completed might be a more accurate choice. >>>>> >>>>> I still stand by this, although I realise no other target does it. >>>> >>>> Did a re-test of the patch just to be sure, as expected the test results >>>> were also clear. Attached is the updated patch. >>> >>> Can you specify what you tested with this patch ? >> >> Native bootstrap success, plus C/C++ and libstdc++ tests. IIRC I also >> saw one or two FAIL->PASS in the results too (forgot specific testcases) >> >>> >>> So, it's interesting to note that the use of this was changed in 2007 by >>> zadeck as a part of the df merge. >>> >>> I can't find the patch trail beyond this on the lists. >>> >>> http://gcc.gnu.org/viewvc/branches/dataflow-branch/gcc/config/arm/arm.h?r1=120281&r2=121501 >>> >>> >>> It might be better to understand why this was done in the first place >>> for the ARM port as part of the Dataflow bring up and why folks wanted >>> to make this unconditional. > > Digging through the repository, this is my explanation, FWIW: > > 1) The gen_prologue_uses() of LR were added back in Dec.2000 (r38467), > when "ce2" was still the if-convert-after-reload pass, placed after > prologue-epilogue construction. (hence the arm_expand_prologue() comment > about preventing ce2 using LR) > > 2) if-conversion after combine was added in Oct.2002 (r58547), which > became the new "ce2" (pre-reload); ifcvt after reload became "ce3". The > comments in arm_expand_prologue() were not updated. > > 3) dataflow-branch work was circa 2007. RTL-ifcvt seemed to be updated > during this time, hence removal of the LR-uses in arm_expand_prologue() > seems reasonable. My guess here: ce2 was mistaken to be > ifcvt-after-combine (rather than the originally intended > ifcvt-after-reload, now ce3) by the comments; considering the > arm_expand_prologue() bits were updated, the comments may have been read > seriously. > > 4) Since "ce2" was a pre-reload pass by then, the unconditionalizing of > EPILOGUE_USES was probably intended to be a supplemental change, to > support removing those gen_prologue_use()s. > > I hope this is a reasonable explanation, but do note a lot of this is > guessing :) > > I tried taking the last version of the dataflow-branch (circa 4.3) and > did cross-test run compares of EPILOGUE_USES with and without the > reload_completed conditionalization. The C testsuite results were clean. > > The LR-not-used symptoms seem triggered by this EPILOGUE_USES change > since then. As the PR42017 submitter lists the affected GCC versions, > this regression has been present since post-4.2. > > Given the above explanation, and considering that the tests on current > trunk are okay, plus we're in stage1 right now, is this > re-conditionalizing EPILOGUE_USES change okay to commit? > > Thanks, > Chung-Lin