Hi Ramana,
On 18 June 2014 15:29, Ramana Radhakrishnan <ramana....@googlemail.com> wrote:
> On Mon, Jun 16, 2014 at 1:53 PM, Venkataramanan Kumar
> <venkataramanan.ku...@linaro.org> wrote:
>> Hi Maintainers,
>>
>> This patch fixes the PR 60617 that occurs when we turn on reload pass
>> in thumb2 mode.
>>
>> It occurs for the pattern "*ior_scc_scc" that gets generated for the 3
>> argument of the below function call.
>>
>> JIT:emitStoreInt32(dst,regT0m, (op1 == dst || op2 == dst)));
>>
>>
>> (----snip---)
>> (insn 634 633 635 27 (parallel [
>> (set (reg:SI 3 r3)
>> (ior:SI (eq:SI (reg/v:SI 110 [ dst ]) <== This operand
>> r5 is registers gets assigned
>> (reg/v:SI 112 [ op2 ]))
>> (eq:SI (reg/v:SI 110 [ dst ]) <== This operand
>> (reg/v:SI 111 [ op1 ]))))
>> (clobber (reg:CC 100 cc))
>> ]) ../Source/JavaScriptCore/jit/JITArithmetic32_64.cpp:179 300
>> {*ior_scc_scc
>> (----snip---)
>>
>> The issue here is that the above pattern demands 5 registers (LO_REGS).
>>
>> But when we are in reload, registers r0 is used for pointer to the
>> class, r1 and r2 for first and second argument. r7 is used for stack
>> pointer.
>>
>> So we are left with r3,r4,r5 and r6. But the above patterns needs five
>> LO_REGS. Hence we get spill failure when processing the last register
>> operand in that pattern,
>>
>> In ARM port, TARGET_LIKELY_SPILLED_CLASS is defined for Thumb-1 and
>> for thumb 2 mode there is mention of using LO_REG in the comment as
>> below.
>>
>> "Care should be taken to avoid adding thumb-2 patterns that require
>> many low registers"
>>
>> So conservative fix is not to allow this pattern for Thumb-2 mode.
>
> I don't have an additional solution off the top of my head and
> probably need to go do some digging.
>
> It sounds like the conservative fix but what's the impact of doing so
> ? Have you measured that in terms of performance or code size on a
> range of benchmarks ?
>
>>
I haven't done any benchmark testing. I will try and run some
benchmarks with my patch.
>> I allowed these pattern for Thumb2 when we have constant operands for
>> comparison. That makes the target tests arm/thum2-cond-cmp-1.c to
>> thum2-cond-cmp-4.c pass.
>
> That sounds fine and fair - no trouble there.
>
> My concern is with removing the register alternatives and loosing the
> ability to trigger conditional compares on 4.9 and trunk for Thumb1
> till the time the "new" conditional compare work makes it in.
>
> Ramana
This bug does not occur when LRA is enabled. In 4.9 FSF and trunk, the
LRA pass is enabled by default now .
May be too conservative, but is there a way to enable this pattern
when we have LRA pass and prevent it we have old reload pass?
regards,
Venkat.
>
>>
>> Regression tested with gcc 4.9 branch since in trunk this bug is
>> masked revision 209897.
>>
>> Please provide your suggestion on this patch
>>
>> regards,
>> Venkat.
