Re: [patch, ARM] Fix PR42017, LR not used in leaf functions

[Chung-Lin Tang <cltang at codesourcery dot com>]

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.sandiford@linaro.org>  writes:
>>>>> Chung-Lin Tang<cltang@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