diff mbox

[ARM] Fix arm_subsi3_insn alternatives

Message ID CAD57uCe0qJmMo_3vv6bChKOV0nLw2OJUtZE2WTLEeUZzW_nEXQ@mail.gmail.com
State New
Headers show

Commit Message

Yvan Roux March 24, 2015, 7:53 p.m. UTC
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.

Comments

Yvan Roux April 13, 2015, 10:47 a.m. UTC | #1
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.
>
>
Yvan Roux April 13, 2015, 10:59 a.m. UTC | #2
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
diff mbox

Patch

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