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<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
