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

[Chung-Lin Tang]

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