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<richard.sandif...@linaro.org> writes: >>>>> Chung-Lin Tang<clt...@codesourcery.com> 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
