Ping^3 Re: [PATCH, ARM] Fix PR44557, Thumb-1 ICE

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

Ping.

On 2011/3/7 10:11 PM, Chung-Lin Tang wrote:
> Ping.
> 
> On 2011/1/26 11:07 AM, Chung-Lin Tang wrote:
>> On 2011/1/1 19:21, Chung-Lin Tang wrote:
>>> On 2010/12/21 02:03, Richard Earnshaw wrote:
>>>>
>>>> On Thu, 2010-12-09 at 14:08 +0800, Chung-Lin Tang wrote:
>>>>> Hi,
>>>>> this patch fixes the ICE in PR44557. It now occurs on trunk only under 
>>>>> quite specific option conditions, but debugging this PR leaded to quite 
>>>>> obvious Thumb-1 fixes.
>>>>>
>>>>> Also added a simplified testcase, derived from the one on bugzilla. 
>>>>> Tested without regressions, okay to commit to trunk?
>>>>>
>>>>> Thanks,
>>>>> Chung-Lin
>>>>>
>>>>> 2010-12-09  Chung-Lin Tang  <cltang@codesourcery.com>
>>>>>
>>>>> 	PR target/44557
>>>>> 	* config/arm/arm.h (PREFERRED_RELOAD_CLASS): Add CORE_REGS to
>>>>> 	Thumb-1 return LO_REGS case.
>>>>> 	* config/arm/arm.md (reload_inhi): Change scratch constraint
>>>>> 	from 'r' to 'l'.
>>>>>
>>>>> 	testsuite/
>>>>> 	* gcc.target/arm/pr44557.c: New.
>>>>>
>>>>
>>>> Changing the scratch constraint to 'l' is going to pessimize thumb2 code
>>>> reload generation which can quite reasonably take an 'r' class here.  
>>>>
>>>> I'm not sure there's an easy way to fix this without going the whole hog
>>>> and getting rid of reload_inhi entirely (which would be a good thing,
>>>> TM) and replacing it with the new reload infrastructure that Joern wrote
>>>> a few years back.
>>>>
>>>> Before approving this I want to be sure that it doesn't cause major
>>>> problems on Thumb-2.  What testing have you done for that configuration?
>>>>
>>>> R.
>>>>
>>>> PS: Maybe another way to fix this would be to introduce a new register
>>>> class that expands to GENERAL_REGS on 32-bit cores and LO_REGS on
>>>> Thumb-1 only.
>>>>
>>>
>>> Hi Richard,
>>>
>>> My testing was for Thumb-1 (-march=armv5te -mthumb), as that was what
>>> the ICE was about. I later tried another test run with Thumb-2 as you
>>> suggested, to be sure, and saw no regressions too.
>>>
>>> I've then been looking into the secondary reload logic. The ARM
>>> backend's SECONDARY_INPUT/OUTPUT_RELOAD_CLASS macros already returns
>>> NO_REGS under TARGET_32BIT for integer modes (no secondary reloads
>>> needed). Looking at the default_secondary_reload() function, this means
>>> that the reload_in/outhi patterns are never used for Thumb-2.
>>>
>>> So this patch should only affect Thumb-1, and the Thumb-2 considerations
>>> should be unneeded.
>>
>> Hi Richard, pinging this patch.
>> I think I've explained the effects of this fix, and it should be Thumb-1
>> only. Is this patch okay?
>>
>> Thanks,
>> Chung-Lin
>