New patch that adds the predicate "reg_or_arm_rhs_operand" as suggested
by Richard. Tested on all arm and thumb targets as before.
Okay for trunk?
2016-03-07 Michael Collison <michael.colli...@linaro.org>
PR target/70008
* config/arm/predicates.md (reg_or_arm_rhs_operand): New predicate
that allows registers or immediates if TARGET_ARM.
* config/arm/arm.md (*subsi3_carryin): Change predicate to
reg_or_arm_rhs_operand to be consistent with constraints.
On 03/03/2016 08:47 PM, Richard Earnshaw (lists) wrote:
On 03/03/16 07:23, Michael Collison wrote:
I have attached a new patch which hopefully address Richard's concerns.
I modified the predicate on operand 1 to to "arm_rhs_operand" to be
consistent with the constraints. I retained the split into two patterns;
one for arm and another for thumb2. I thought this was cleaner.
Okay for trunk?
2016-02-28 Michael Collison <michael.colli...@linaro.org>
PR target/70008
* config/arm/arm.md (*subsi3_carryin): Change predicate to
arm_rhs_operand to be consistent with constraints.
Only allow pattern for TARGET_ARM.
* config/arm/thumb2.md (*thumb2_subsi3_carryin): New pattern.
I don't think we need two patterns. We could share this if we had a new
predicate that was called something like reg_or_arm_rhs_operand, with a
rule that's something like:
register_operand (op) || (TARGET_ARM && arm_immediate_operand (op))
R.
On 02/29/2016 08:29 AM, Richard Earnshaw (lists) wrote:
On 29/02/16 11:21, Michael Collison wrote:
On 2/29/2016 4:06 AM, Kyrill Tkachov wrote:
Hi Michael,
On 29/02/16 04:47, Michael Collison wrote:
This patches address PR 70008, where a reverse subtract with carry
instruction can be generated in thumb2 mode. It was tested with no
regressions in arm and thumb modes on the following targets:
arm-none-linux-gnueabi
arm-none-linux-gnuabihf
armeb-none-linux-gnuabihf
arm-none-eabi
Okay for trunk?
2016-02-28 Michael Collison <michael.colli...@linaro.org>
PR target/70008
* config/arm/arm.md (*subsi3_carryin): Only match pattern if
TARGET_ARM due to 'rsc' instruction alternative.
* config/arm/thumb2.md (*thumb2_subsi3_carryin): New pattern.
The *subsi3_carrying pattern has the arch attribute:
(set_attr "arch" "*,a")
That means that the second alternative that generates the RSC
instruction is only enabled
for ARM mode. Do you have a testcase where this doesn't happen and
this pattern generates
the second alternative for Thumb2?
No I don't have a test case; i noticed the pattern when working on the
overflow project. I did not realize
that an attribute could affect the matching of an alternative. I will
close the bug.
Thanks,
Kyrill
This is all true, but there is a potential performance issue with this
pattern though, that could lead to sub-optimal code.
The predicate accepts reg-or-int, but in ARM state only simple
'const-ok-for-arm' immediates are permitted by the predicates, and in
thumb code, no immediates are permitted at all. This could potentially
result in sub-optimal code due to late splitting of the pattern. It
would be better if the predicate understood these limitations and
restricted immediates accordingly.
R.
bugzilla-70008-upstream-v2.patch
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index e67239d..e6bcd7f 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -867,15 +867,14 @@
(define_insn "*subsi3_carryin"
[(set (match_operand:SI 0 "s_register_operand" "=r,r")
- (minus:SI (minus:SI (match_operand:SI 1 "reg_or_int_operand" "r,I")
+ (minus:SI (minus:SI (match_operand:SI 1 "arm_rhs_operand" "r,I")
(match_operand:SI 2 "s_register_operand" "r,r"))
(ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
- "TARGET_32BIT"
+ "TARGET_ARM"
"@
sbc%?\\t%0, %1, %2
rsc%?\\t%0, %2, %1"
[(set_attr "conds" "use")
- (set_attr "arch" "*,a")
(set_attr "predicable" "yes")
(set_attr "predicable_short_it" "no")
(set_attr "type" "adc_reg,adc_imm")]
diff --git a/gcc/config/arm/thumb2.md b/gcc/config/arm/thumb2.md
index 9925365..79305c5 100644
--- a/gcc/config/arm/thumb2.md
+++ b/gcc/config/arm/thumb2.md
@@ -848,6 +848,20 @@
(set_attr "type" "multiple")]
)
+(define_insn "*thumb2_subsi3_carryin"
+ [(set (match_operand:SI 0 "s_register_operand" "=r")
+ (minus:SI (minus:SI (match_operand:SI 1 "s_register_operand" "r")
+ (match_operand:SI 2 "s_register_operand" "r"))
+ (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+ "TARGET_THUMB2"
+ "@
+ sbc%?\\t%0, %1, %2"
+ [(set_attr "conds" "use")
+ (set_attr "predicable" "yes")
+ (set_attr "predicable_short_it" "no")
+ (set_attr "type" "adc_reg")]
+)
+
(define_insn "*thumb2_cond_sub"
[(set (match_operand:SI 0 "s_register_operand" "=Ts,Ts")
(minus:SI (match_operand:SI 1 "s_register_operand" "0,?Ts")
--
Michael Collison
Linaro Toolchain Working Group
michael.colli...@linaro.org
diff --git a/gcc/config/arm/arm.md b/gcc/config/arm/arm.md
index e67239d..eb4803a 100644
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -867,7 +867,7 @@
(define_insn "*subsi3_carryin"
[(set (match_operand:SI 0 "s_register_operand" "=r,r")
- (minus:SI (minus:SI (match_operand:SI 1 "reg_or_int_operand" "r,I")
+ (minus:SI (minus:SI (match_operand:SI 1 "reg_or_arm_rhs_operand" "r,I")
(match_operand:SI 2 "s_register_operand" "r,r"))
(ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
"TARGET_32BIT"
diff --git a/gcc/config/arm/predicates.md b/gcc/config/arm/predicates.md
index b1cd556..8bf7c1a 100644
--- a/gcc/config/arm/predicates.md
+++ b/gcc/config/arm/predicates.md
@@ -144,6 +144,11 @@
(and (match_code "const_int")
(match_test "INTVAL (op) == 0")))
+(define_predicate "reg_or_arm_rhs_operand"
+ (ior (match_operand 0 "s_register_operand")
+ (and (match_test "TARGET_ARM")
+ (match_operand 0 "arm_immediate_operand"))))
+
;; Something valid on the RHS of an ARM data-processing instruction
(define_predicate "arm_rhs_operand"
(ior (match_operand 0 "s_register_operand")
--
1.9.1
