[PATCH, ARM] Fix arm_subsi3_insn alternatives

[PATCH, ARM] Fix arm_subsi3_insn alternatives

[Yvan Roux]

[-- Attachment #1: Type: text/plain, Size: 679 bytes --]

Hi,

after the issue with duplicated alternatives exhibited by PR64208, I
checked the arm.md file and found that *arm_subsi3_insn has a
duplication where alt 4 is (r,rI,r) and alt 6 is (r,r,r), this results
in emitting an rsb instruction instead of a sub one, but it has also
an impact on scheduling as the type attribute affected to alt 4 is
alu_imm when it could only involve registers.

This is fixed by this small patch. Cross builded and regtested for
arm/armeb targets.
Ok for trunk (maybe for stage 1 as no PR is attached to that) ?

Cheers,
Yvan

 2105-03-24  Yvan Roux  <yvan.roux@linaro.org>

    * config/arm/arm.md ("*arm_subsi3_insn"): Fixed redundant alternatives.

[-- Attachment #2: subsi3.diff --]
[-- Type: text/plain, Size: 778 bytes --]

diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index 164ac13..b4e50c2 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -1177,9 +1177,9 @@
 
 ; ??? Check Thumb-2 split length
 (define_insn_and_split "*arm_subsi3_insn"
-  [(set (match_operand:SI           0 "s_register_operand" "=l,l ,l ,l ,r ,r,r,rk,r")
-	(minus:SI (match_operand:SI 1 "reg_or_int_operand" "l ,0 ,l ,Pz,rI,r,r,k ,?n")
-		  (match_operand:SI 2 "reg_or_int_operand" "l ,Py,Pd,l ,r ,I,r,r ,r")))]
+  [(set (match_operand:SI           0 "s_register_operand" "=l,l ,l ,l ,r,r,r,rk,r")
+	(minus:SI (match_operand:SI 1 "reg_or_int_operand" "l ,0 ,l ,Pz,I,r,r,k ,?n")
+		  (match_operand:SI 2 "reg_or_int_operand" "l ,Py,Pd,l ,r,I,r,r ,r")))]
   "TARGET_32BIT"
   "@
    sub%?\\t%0, %1, %2

Re: [PATCH, ARM] Fix arm_subsi3_insn alternatives

[Kyrill Tkachov]

On 24/03/15 19:53, Yvan Roux wrote:
> Hi,
>
> after the issue with duplicated alternatives exhibited by PR64208, I
> checked the arm.md file and found that *arm_subsi3_insn has a
> duplication where alt 4 is (r,rI,r) and alt 6 is (r,r,r), this results
> in emitting an rsb instruction instead of a sub one, but it has also
> an impact on scheduling as the type attribute affected to alt 4 is
> alu_imm when it could only involve registers.
>
> This is fixed by this small patch. Cross builded and regtested for
> arm/armeb targets.
> Ok for trunk (maybe for stage 1 as no PR is attached to that) ?

Hi Yvan,

I've kicked off two bootstraps of this patch on arm-none-linux-gnueabihf:
one with arm mode and one with thumb. Both are well in stage3 so I don't 
expect
there to be any problems and the patch looks ok to me code-wise (though
I can't approve).

This is stage 1 material by now though since there's no regression 
associated.

Cheers,
Kyrill

>
> Cheers,
> Yvan
>
>   2105-03-24  Yvan Roux  <yvan.roux@linaro.org>
>
>      * config/arm/arm.md ("*arm_subsi3_insn"): Fixed redundant alternatives.

Re: [PATCH, ARM] Fix arm_subsi3_insn alternatives

[Yvan Roux]

Ping (now that stage1 is open)

On 27 March 2015 at 19:14, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>
> On 24/03/15 19:53, Yvan Roux wrote:
>>
>> Hi,
>>
>> after the issue with duplicated alternatives exhibited by PR64208, I
>> checked the arm.md file and found that *arm_subsi3_insn has a
>> duplication where alt 4 is (r,rI,r) and alt 6 is (r,r,r), this results
>> in emitting an rsb instruction instead of a sub one, but it has also
>> an impact on scheduling as the type attribute affected to alt 4 is
>> alu_imm when it could only involve registers.
>>
>> This is fixed by this small patch. Cross builded and regtested for
>> arm/armeb targets.
>> Ok for trunk (maybe for stage 1 as no PR is attached to that) ?
>
>
> Hi Yvan,
>
> I've kicked off two bootstraps of this patch on arm-none-linux-gnueabihf:
> one with arm mode and one with thumb. Both are well in stage3 so I don't
> expect
> there to be any problems and the patch looks ok to me code-wise (though
> I can't approve).
>
> This is stage 1 material by now though since there's no regression
> associated.
>
> Cheers,
> Kyrill
>
>
>>
>> Cheers,
>> Yvan
>>
>>   2105-03-24  Yvan Roux  <yvan.roux@linaro.org>
>>
>>      * config/arm/arm.md ("*arm_subsi3_insn"): Fixed redundant
>> alternatives.
>
>

Re: [PATCH, ARM] Fix arm_subsi3_insn alternatives

[Kyrill Tkachov]

On 13/04/15 11:47, Yvan Roux wrote:
> Ping (now that stage1 is open)
Hi Yvan,

This patch is ok, but can you wait until GCC 5.1 is released before committing please?
That way trunk and the GCC 5 branch won't diverge too much if there are any last minute
(hopefully none in this area!) fixes that need to go in.


Thanks,
Kyrill

>
> On 27 March 2015 at 19:14, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>> On 24/03/15 19:53, Yvan Roux wrote:
>>> Hi,
>>>
>>> after the issue with duplicated alternatives exhibited by PR64208, I
>>> checked the arm.md file and found that *arm_subsi3_insn has a
>>> duplication where alt 4 is (r,rI,r) and alt 6 is (r,r,r), this results
>>> in emitting an rsb instruction instead of a sub one, but it has also
>>> an impact on scheduling as the type attribute affected to alt 4 is
>>> alu_imm when it could only involve registers.
>>>
>>> This is fixed by this small patch. Cross builded and regtested for
>>> arm/armeb targets.
>>> Ok for trunk (maybe for stage 1 as no PR is attached to that) ?
>>
>> Hi Yvan,
>>
>> I've kicked off two bootstraps of this patch on arm-none-linux-gnueabihf:
>> one with arm mode and one with thumb. Both are well in stage3 so I don't
>> expect
>> there to be any problems and the patch looks ok to me code-wise (though
>> I can't approve).
>>
>> This is stage 1 material by now though since there's no regression
>> associated.
>>
>> Cheers,
>> Kyrill
>>
>>
>>> Cheers,
>>> Yvan
>>>
>>>    2105-03-24  Yvan Roux  <yvan.roux@linaro.org>
>>>
>>>       * config/arm/arm.md ("*arm_subsi3_insn"): Fixed redundant
>>> alternatives.
>>

Re: [PATCH, ARM] Fix arm_subsi3_insn alternatives

[Yvan Roux]

On 13 April 2015 at 12:55, Kyrill Tkachov <kyrylo.tkachov@arm.com> wrote:
>
> On 13/04/15 11:47, Yvan Roux wrote:
>>
>> Ping (now that stage1 is open)
>
> Hi Yvan,
>
> This patch is ok, but can you wait until GCC 5.1 is released before
> committing please?
> That way trunk and the GCC 5 branch won't diverge too much if there are any
> last minute
> (hopefully none in this area!) fixes that need to go in.

Ok no problem. Thanks Kyrill

Cheers,
Yvan