* [PATCH][ARM] Add support for ADDW and SUBW instructions
@ 2011-04-20 15:36 Andrew Stubbs
2011-04-20 15:56 ` Andrew Stubbs
` (3 more replies)
0 siblings, 4 replies; 16+ messages in thread
From: Andrew Stubbs @ 2011-04-20 15:36 UTC (permalink / raw)
To: gcc-patches; +Cc: patches
[-- Attachment #1: Type: text/plain, Size: 405 bytes --]
This patch adds basic support for the Thumb ADDW and SUBW instructions.
The patch permits the compiler to use the new instructions for constants
that can be loaded with a single instruction (i.e. 16-bit unshifted),
but does not support use of addw with split-constants; I have a patch
for that coming soon.
This patch requires that my previously posted patch for MOVW is applied
first.
OK?
Andrew
[-- Attachment #2: addwsubw.patch --]
[-- Type: text/x-patch, Size: 6256 bytes --]
2011-04-20 Andrew Stubbs <ams@codesourcery.com>
gcc/
* config/arm/arm-protos.h (const_ok_for_op): Add prototype.
* config/arm/arm.c (const_ok_for_op): Add support for addw/subw.
Remove prototype. Remove static function type.
* config/arm/arm.md (*arm_addsi3): Add addw/subw support.
Add arch attribute.
(*arm_subsi3_insn): Add subw support.
Add arch attribute.
* config/arm/constraints.md (Pj, PJ): New constraints.
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -46,6 +46,7 @@ extern bool arm_vector_mode_supported_p (enum machine_mode);
extern bool arm_small_register_classes_for_mode_p (enum machine_mode);
extern int arm_hard_regno_mode_ok (unsigned int, enum machine_mode);
extern int const_ok_for_arm (HOST_WIDE_INT);
+extern int const_ok_for_op (HOST_WIDE_INT, enum rtx_code);
extern int arm_split_constant (RTX_CODE, enum machine_mode, rtx,
HOST_WIDE_INT, rtx, rtx, int);
extern RTX_CODE arm_canonicalize_comparison (RTX_CODE, rtx *, rtx *);
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -82,7 +82,6 @@ inline static int thumb1_index_register_rtx_p (rtx, int);
static bool arm_legitimate_address_p (enum machine_mode, rtx, bool);
static int thumb_far_jump_used_p (void);
static bool thumb_force_lr_save (void);
-static int const_ok_for_op (HOST_WIDE_INT, enum rtx_code);
static rtx emit_sfm (int, int);
static unsigned arm_size_return_regs (void);
static bool arm_assemble_integer (rtx, unsigned int, int);
@@ -2453,7 +2452,7 @@ const_ok_for_arm (HOST_WIDE_INT i)
}
/* Return true if I is a valid constant for the operation CODE. */
-static int
+int
const_ok_for_op (HOST_WIDE_INT i, enum rtx_code code)
{
if (const_ok_for_arm (i))
@@ -2469,6 +2468,13 @@ const_ok_for_op (HOST_WIDE_INT i, enum rtx_code code)
return 0;
case PLUS:
+ /* See if we can use addw or subw. */
+ if (TARGET_THUMB2
+ && ((i & 0xfffff000) == 0
+ || ((-i) & 0xfffff000) == 0))
+ return 1;
+ /* else fall through. */
+
case COMPARE:
case EQ:
case NE:
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -707,21 +707,24 @@
;; (plus (reg rN) (reg sp)) into (reg rN). In this case reload will
;; put the duplicated register first, and not try the commutative version.
(define_insn_and_split "*arm_addsi3"
- [(set (match_operand:SI 0 "s_register_operand" "=r, k,r,r, k,r")
- (plus:SI (match_operand:SI 1 "s_register_operand" "%rk,k,r,rk,k,rk")
- (match_operand:SI 2 "reg_or_int_operand" "rI,rI,k,L, L,?n")))]
+ [(set (match_operand:SI 0 "s_register_operand" "=r, k,r,r, k, r, k,r, k, r")
+ (plus:SI (match_operand:SI 1 "s_register_operand" "%rk,k,r,rk,k, rk,k,rk,k, rk")
+ (match_operand:SI 2 "reg_or_int_operand" "rI,rI,k,Pj,Pj,L, L,PJ,PJ,?n")))]
"TARGET_32BIT"
"@
add%?\\t%0, %1, %2
add%?\\t%0, %1, %2
add%?\\t%0, %2, %1
+ addw%?\\t%0, %1, %2
+ addw%?\\t%0, %1, %2
sub%?\\t%0, %1, #%n2
sub%?\\t%0, %1, #%n2
+ subw%?\\t%0, %1, #%n2
+ subw%?\\t%0, %1, #%n2
#"
"TARGET_32BIT
&& GET_CODE (operands[2]) == CONST_INT
- && !(const_ok_for_arm (INTVAL (operands[2]))
- || const_ok_for_arm (-INTVAL (operands[2])))
+ && !const_ok_for_op (INTVAL (operands[2]), PLUS)
&& (reload_completed || !arm_eliminable_register (operands[1]))"
[(clobber (const_int 0))]
"
@@ -730,8 +733,9 @@
operands[1], 0);
DONE;
"
- [(set_attr "length" "4,4,4,4,4,16")
- (set_attr "predicable" "yes")]
+ [(set_attr "length" "4,4,4,4,4,4,4,4,4,16")
+ (set_attr "predicable" "yes")
+ (set_attr "arch" "*,*,*,t2,t2,*,*,t2,t2,*")]
)
(define_insn_and_split "*thumb1_addsi3"
@@ -1184,28 +1188,33 @@
; ??? Check Thumb-2 split length
(define_insn_and_split "*arm_subsi3_insn"
- [(set (match_operand:SI 0 "s_register_operand" "=r,r,rk,r,r")
- (minus:SI (match_operand:SI 1 "reg_or_int_operand" "rI,r,k,?n,r")
- (match_operand:SI 2 "reg_or_int_operand" "r,rI,r, r,?n")))]
+ [(set (match_operand:SI 0 "s_register_operand" "=r,r,rk,r, k, r,r")
+ (minus:SI (match_operand:SI 1 "reg_or_int_operand" "rI,r,k, rk,k, ?n,r")
+ (match_operand:SI 2 "reg_or_int_operand" "r,rI,r, Pj,Pj,r,?n")))]
"TARGET_32BIT"
"@
rsb%?\\t%0, %2, %1
sub%?\\t%0, %1, %2
sub%?\\t%0, %1, %2
+ subw%?\\t%0, %1, %2
+ subw%?\\t%0, %1, %2
#
#"
"&& ((GET_CODE (operands[1]) == CONST_INT
- && !const_ok_for_arm (INTVAL (operands[1])))
+ && !(const_ok_for_arm (INTVAL (operands[1]))
+ || satisfies_constraint_Pj (operands[2])))
|| (GET_CODE (operands[2]) == CONST_INT
- && !const_ok_for_arm (INTVAL (operands[2]))))"
+ && !(const_ok_for_arm (INTVAL (operands[2]))
+ || satisfies_constraint_Pj (operands[2]))))"
[(clobber (const_int 0))]
"
arm_split_constant (MINUS, SImode, curr_insn,
INTVAL (operands[1]), operands[0], operands[2], 0);
DONE;
"
- [(set_attr "length" "4,4,4,16,16")
- (set_attr "predicable" "yes")]
+ [(set_attr "length" "4,4,4,4,4,16,16")
+ (set_attr "predicable" "yes")
+ (set_attr "arch" "*,*,*,t2,t2,*,*")]
)
(define_peephole2
--- a/gcc/config/arm/constraints.md
+++ b/gcc/config/arm/constraints.md
@@ -31,7 +31,7 @@
;; The following multi-letter normal constraints have been used:
;; in ARM/Thumb-2 state: Da, Db, Dc, Dn, Dl, DL, Dv, Dy, Di, Dz
;; in Thumb-1 state: Pa, Pb, Pc, Pd
-;; in Thumb-2 state: Ps, Pt, Pu, Pv, Pw, Px, Py
+;; in Thumb-2 state: Pj, PJ, Ps, Pt, Pu, Pv, Pw, Px, Py
;; The following memory constraints have been used:
;; in ARM/Thumb-2 state: Q, Ut, Uv, Uy, Un, Um, Us
@@ -74,6 +74,18 @@
(and (match_code "const_int")
(match_test "(ival & 0xffff0000) == 0")))))
+(define_constraint "Pj"
+ "A 12-bit constant suitable for an ADDW or SUBW instruction. (Thumb-2)"
+ (and (match_code "const_int")
+ (and (match_test "TARGET_THUMB2")
+ (match_test "(ival & 0xfffff000) == 0"))))
+
+(define_constraint "PJ"
+ "A constant that satisfies the Pj constrant if negated."
+ (and (match_code "const_int")
+ (and (match_test "TARGET_THUMB2")
+ (match_test "((-ival) & 0xfffff000) == 0"))))
+
(define_register_constraint "k" "STACK_REG"
"@internal The stack register.")
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
2011-04-20 15:36 [PATCH][ARM] Add support for ADDW and SUBW instructions Andrew Stubbs
@ 2011-04-20 15:56 ` Andrew Stubbs
2011-05-18 15:47 ` Andrew Stubbs
` (2 subsequent siblings)
3 siblings, 0 replies; 16+ messages in thread
From: Andrew Stubbs @ 2011-04-20 15:56 UTC (permalink / raw)
Cc: gcc-patches, patches
On 20/04/11 16:27, Andrew Stubbs wrote:
> (*arm_subsi3_insn): Add subw support.
Oh, I should probably say that I've added subw support to arm_subsi3
even though it's not obvious that anything will ever use this.
The existing implementation of arm_subsi3 (sans 'w') supports
immediates, so I added subw to match.
If there are any objections, I expect I can remove that hunk of the patch.
Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
2011-04-20 15:36 [PATCH][ARM] Add support for ADDW and SUBW instructions Andrew Stubbs
2011-04-20 15:56 ` Andrew Stubbs
@ 2011-05-18 15:47 ` Andrew Stubbs
2011-06-02 8:07 ` Andrew Stubbs
2011-06-02 8:24 ` Ramana Radhakrishnan
3 siblings, 0 replies; 16+ messages in thread
From: Andrew Stubbs @ 2011-05-18 15:47 UTC (permalink / raw)
Cc: gcc-patches, patches
Ping.
On 20/04/11 16:27, Andrew Stubbs wrote:
> This patch adds basic support for the Thumb ADDW and SUBW instructions.
>
> The patch permits the compiler to use the new instructions for constants
> that can be loaded with a single instruction (i.e. 16-bit unshifted),
> but does not support use of addw with split-constants; I have a patch
> for that coming soon.
>
> This patch requires that my previously posted patch for MOVW is applied
> first.
>
> OK?
>
> Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
2011-04-20 15:36 [PATCH][ARM] Add support for ADDW and SUBW instructions Andrew Stubbs
2011-04-20 15:56 ` Andrew Stubbs
2011-05-18 15:47 ` Andrew Stubbs
@ 2011-06-02 8:07 ` Andrew Stubbs
2011-06-02 8:24 ` Ramana Radhakrishnan
3 siblings, 0 replies; 16+ messages in thread
From: Andrew Stubbs @ 2011-06-02 8:07 UTC (permalink / raw)
Cc: gcc-patches, patches
Ping 2.
On 20/04/11 16:27, Andrew Stubbs wrote:
> This patch adds basic support for the Thumb ADDW and SUBW instructions.
>
> The patch permits the compiler to use the new instructions for constants
> that can be loaded with a single instruction (i.e. 16-bit unshifted),
> but does not support use of addw with split-constants; I have a patch
> for that coming soon.
>
> This patch requires that my previously posted patch for MOVW is applied
> first.
>
> OK?
>
> Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
2011-04-20 15:36 [PATCH][ARM] Add support for ADDW and SUBW instructions Andrew Stubbs
` (2 preceding siblings ...)
2011-06-02 8:07 ` Andrew Stubbs
@ 2011-06-02 8:24 ` Ramana Radhakrishnan
2011-06-02 9:03 ` Andrew Stubbs
3 siblings, 1 reply; 16+ messages in thread
From: Ramana Radhakrishnan @ 2011-06-02 8:24 UTC (permalink / raw)
To: Andrew Stubbs; +Cc: gcc-patches, patches
> OK?
>
This is largely OK modulo the following.
Please remove the alternatives in the subsi3 pattern since that is just
unnecessary. Please make the constraints internal only.
cheers
Ramana
> Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
2011-06-02 8:24 ` Ramana Radhakrishnan
@ 2011-06-02 9:03 ` Andrew Stubbs
2011-06-02 10:37 ` Ramana Radhakrishnan
2011-06-06 12:15 ` Dmitry Plotnikov
0 siblings, 2 replies; 16+ messages in thread
From: Andrew Stubbs @ 2011-06-02 9:03 UTC (permalink / raw)
To: Ramana Radhakrishnan; +Cc: gcc-patches, patches
[-- Attachment #1: Type: text/plain, Size: 204 bytes --]
On 02/06/11 09:23, Ramana Radhakrishnan wrote:
> Please remove the alternatives in the subsi3 pattern since that is just
> unnecessary. Please make the constraints internal only.
Is this better?
Andrew
[-- Attachment #2: addwsubw.patch --]
[-- Type: text/x-patch, Size: 4730 bytes --]
2011-06-02 Andrew Stubbs <ams@codesourcery.com>
gcc/
* config/arm/arm-protos.h (const_ok_for_op): Add prototype.
* config/arm/arm.c (const_ok_for_op): Add support for addw/subw.
Remove prototype. Remove static function type.
* config/arm/arm.md (*arm_addsi3): Add addw/subw support.
Add arch attribute.
* config/arm/constraints.md (Pj, PJ): New constraints.
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -46,6 +46,7 @@ extern bool arm_vector_mode_supported_p (enum machine_mode);
extern bool arm_small_register_classes_for_mode_p (enum machine_mode);
extern int arm_hard_regno_mode_ok (unsigned int, enum machine_mode);
extern int const_ok_for_arm (HOST_WIDE_INT);
+extern int const_ok_for_op (HOST_WIDE_INT, enum rtx_code);
extern int arm_split_constant (RTX_CODE, enum machine_mode, rtx,
HOST_WIDE_INT, rtx, rtx, int);
extern RTX_CODE arm_canonicalize_comparison (RTX_CODE, rtx *, rtx *);
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -82,7 +82,6 @@ inline static int thumb1_index_register_rtx_p (rtx, int);
static bool arm_legitimate_address_p (enum machine_mode, rtx, bool);
static int thumb_far_jump_used_p (void);
static bool thumb_force_lr_save (void);
-static int const_ok_for_op (HOST_WIDE_INT, enum rtx_code);
static rtx emit_sfm (int, int);
static unsigned arm_size_return_regs (void);
static bool arm_assemble_integer (rtx, unsigned int, int);
@@ -2149,7 +2148,7 @@ const_ok_for_arm (HOST_WIDE_INT i)
}
/* Return true if I is a valid constant for the operation CODE. */
-static int
+int
const_ok_for_op (HOST_WIDE_INT i, enum rtx_code code)
{
if (const_ok_for_arm (i))
@@ -2165,6 +2164,13 @@ const_ok_for_op (HOST_WIDE_INT i, enum rtx_code code)
return 0;
case PLUS:
+ /* See if we can use addw or subw. */
+ if (TARGET_THUMB2
+ && ((i & 0xfffff000) == 0
+ || ((-i) & 0xfffff000) == 0))
+ return 1;
+ /* else fall through. */
+
case COMPARE:
case EQ:
case NE:
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -707,21 +707,24 @@
;; (plus (reg rN) (reg sp)) into (reg rN). In this case reload will
;; put the duplicated register first, and not try the commutative version.
(define_insn_and_split "*arm_addsi3"
- [(set (match_operand:SI 0 "s_register_operand" "=r, k,r,r, k,r")
- (plus:SI (match_operand:SI 1 "s_register_operand" "%rk,k,r,rk,k,rk")
- (match_operand:SI 2 "reg_or_int_operand" "rI,rI,k,L, L,?n")))]
+ [(set (match_operand:SI 0 "s_register_operand" "=r, k,r,r, k, r, k,r, k, r")
+ (plus:SI (match_operand:SI 1 "s_register_operand" "%rk,k,r,rk,k, rk,k,rk,k, rk")
+ (match_operand:SI 2 "reg_or_int_operand" "rI,rI,k,Pj,Pj,L, L,PJ,PJ,?n")))]
"TARGET_32BIT"
"@
add%?\\t%0, %1, %2
add%?\\t%0, %1, %2
add%?\\t%0, %2, %1
+ addw%?\\t%0, %1, %2
+ addw%?\\t%0, %1, %2
sub%?\\t%0, %1, #%n2
sub%?\\t%0, %1, #%n2
+ subw%?\\t%0, %1, #%n2
+ subw%?\\t%0, %1, #%n2
#"
"TARGET_32BIT
&& GET_CODE (operands[2]) == CONST_INT
- && !(const_ok_for_arm (INTVAL (operands[2]))
- || const_ok_for_arm (-INTVAL (operands[2])))
+ && !const_ok_for_op (INTVAL (operands[2]), PLUS)
&& (reload_completed || !arm_eliminable_register (operands[1]))"
[(clobber (const_int 0))]
"
@@ -730,8 +733,9 @@
operands[1], 0);
DONE;
"
- [(set_attr "length" "4,4,4,4,4,16")
- (set_attr "predicable" "yes")]
+ [(set_attr "length" "4,4,4,4,4,4,4,4,4,16")
+ (set_attr "predicable" "yes")
+ (set_attr "arch" "*,*,*,t2,t2,*,*,t2,t2,*")]
)
(define_insn_and_split "*thumb1_addsi3"
--- a/gcc/config/arm/constraints.md
+++ b/gcc/config/arm/constraints.md
@@ -31,7 +31,7 @@
;; The following multi-letter normal constraints have been used:
;; in ARM/Thumb-2 state: Da, Db, Dc, Dn, Dl, DL, Dv, Dy, Di, Dz
;; in Thumb-1 state: Pa, Pb, Pc, Pd
-;; in Thumb-2 state: Ps, Pt, Pu, Pv, Pw, Px, Py
+;; in Thumb-2 state: Pj, PJ, Ps, Pt, Pu, Pv, Pw, Px, Py
;; The following memory constraints have been used:
;; in ARM/Thumb-2 state: Q, Ut, Uv, Uy, Un, Um, Us
@@ -75,6 +75,18 @@
(and (match_code "const_int")
(match_test "(ival & 0xffff0000) == 0")))))
+(define_constraint "Pj"
+ "@internal A 12-bit constant suitable for an ADDW or SUBW instruction. (Thumb-2)"
+ (and (match_code "const_int")
+ (and (match_test "TARGET_THUMB2")
+ (match_test "(ival & 0xfffff000) == 0"))))
+
+(define_constraint "PJ"
+ "@internal A constant that satisfies the Pj constrant if negated."
+ (and (match_code "const_int")
+ (and (match_test "TARGET_THUMB2")
+ (match_test "((-ival) & 0xfffff000) == 0"))))
+
(define_register_constraint "k" "STACK_REG"
"@internal The stack register.")
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
2011-06-02 9:03 ` Andrew Stubbs
@ 2011-06-02 10:37 ` Ramana Radhakrishnan
2011-06-16 9:41 ` Stubbs, Andrew
2011-06-06 12:15 ` Dmitry Plotnikov
1 sibling, 1 reply; 16+ messages in thread
From: Ramana Radhakrishnan @ 2011-06-02 10:37 UTC (permalink / raw)
To: Andrew Stubbs; +Cc: gcc-patches, patches
On 2 June 2011 10:03, Andrew Stubbs <ams@codesourcery.com> wrote:
> On 02/06/11 09:23, Ramana Radhakrishnan wrote:
>>
>> Please remove the alternatives in the subsi3 pattern since that is just
>> unnecessary. Please make the constraints internal only.
>
> Is this better?
>
OK.
Ramana
> Andrew
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
2011-06-02 9:03 ` Andrew Stubbs
2011-06-02 10:37 ` Ramana Radhakrishnan
@ 2011-06-06 12:15 ` Dmitry Plotnikov
2011-06-06 12:41 ` Andrew Stubbs
1 sibling, 1 reply; 16+ messages in thread
From: Dmitry Plotnikov @ 2011-06-06 12:15 UTC (permalink / raw)
To: Andrew Stubbs; +Cc: Ramana Radhakrishnan, gcc-patches, patches, dm
[-- Attachment #1: Type: text/plain, Size: 164 bytes --]
This is follow-up patch that fixes rtx costs for CONST_INT in PLUS
pattern. Original discussion is here:
http://gcc.gnu.org/ml/gcc-patches/2011-05/msg01427.html
[-- Attachment #2: costplus.patch --]
[-- Type: text/x-patch, Size: 768 bytes --]
2011-06-06 Dmitry PLotnikov <dplotnikov@ispras.ru>
gcc/
* config/arm/arm.c (arm_rtx_costs_1): Fixed costs for CONST_INT
in PLUS pattern.
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 22ddcd2..9ef6f6d 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -7050,7 +7056,10 @@ arm_rtx_costs_1 (rtx x, enum rtx_code outer, int* total, bool speed)
case CONST_INT:
if (const_ok_for_arm (INTVAL (x))
- || const_ok_for_arm (~INTVAL (x)))
+ || const_ok_for_arm (~INTVAL (x))
+ || (TARGET_THUMB2 && outer == PLUS
+ && (const_ok_for_op (INTVAL (x), outer)
+ || const_ok_for_op (~INTVAL (x), outer))))
*total = COSTS_N_INSNS (1);
else
*total = COSTS_N_INSNS (arm_gen_constant (SET, mode, NULL_RTX,
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
2011-06-06 12:15 ` Dmitry Plotnikov
@ 2011-06-06 12:41 ` Andrew Stubbs
2011-06-06 13:26 ` Dmitry Plotnikov
0 siblings, 1 reply; 16+ messages in thread
From: Andrew Stubbs @ 2011-06-06 12:41 UTC (permalink / raw)
To: Dmitry Plotnikov; +Cc: Ramana Radhakrishnan, gcc-patches, patches, dm
On 06/06/11 13:15, Dmitry Plotnikov wrote:
> + && (const_ok_for_op (INTVAL (x), outer)
> + || const_ok_for_op (~INTVAL (x), outer))))
The second call is redundant. const_ok_for_op should already do that.
Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
2011-06-06 12:41 ` Andrew Stubbs
@ 2011-06-06 13:26 ` Dmitry Plotnikov
2011-06-06 13:33 ` Andrew Stubbs
0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Plotnikov @ 2011-06-06 13:26 UTC (permalink / raw)
To: Andrew Stubbs; +Cc: Ramana Radhakrishnan, gcc-patches, patches, dm
[-- Attachment #1: Type: text/plain, Size: 303 bytes --]
On 06/06/2011 04:41 PM, Andrew Stubbs wrote:
> On 06/06/11 13:15, Dmitry Plotnikov wrote:
>> + && (const_ok_for_op (INTVAL (x), outer)
>> + || const_ok_for_op (~INTVAL (x), outer))))
>
> The second call is redundant. const_ok_for_op should already do that.
>
Fixed patch is attached. Ok?
[-- Attachment #2: costplus.patch --]
[-- Type: text/x-patch, Size: 715 bytes --]
2011-06-06 Dmitry PLotnikov <dplotnikov@ispras.ru>
gcc/
* config/arm/arm.c (arm_rtx_costs_1): Fixed costs for CONST_INT
in PLUS pattern.
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 22ddcd2..9ef6f6d 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -7050,7 +7056,9 @@ arm_rtx_costs_1 (rtx x, enum rtx_code outer, int* total, bool speed)
case CONST_INT:
if (const_ok_for_arm (INTVAL (x))
- || const_ok_for_arm (~INTVAL (x)))
+ || const_ok_for_arm (~INTVAL (x))
+ || (TARGET_THUMB2 && outer == PLUS
+ && (const_ok_for_op (INTVAL (x), outer))))
*total = COSTS_N_INSNS (1);
else
*total = COSTS_N_INSNS (arm_gen_constant (SET, mode, NULL_RTX,
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
2011-06-06 13:26 ` Dmitry Plotnikov
@ 2011-06-06 13:33 ` Andrew Stubbs
2011-06-06 14:26 ` Dmitry Plotnikov
0 siblings, 1 reply; 16+ messages in thread
From: Andrew Stubbs @ 2011-06-06 13:33 UTC (permalink / raw)
To: Dmitry Plotnikov; +Cc: Ramana Radhakrishnan, gcc-patches, patches, dm
On 06/06/11 14:26, Dmitry Plotnikov wrote:
> if (const_ok_for_arm (INTVAL (x))
> - || const_ok_for_arm (~INTVAL (x)))
> + || const_ok_for_arm (~INTVAL (x))
> + || (TARGET_THUMB2&& outer == PLUS
> + && (const_ok_for_op (INTVAL (x), outer))))
Sorry, I should have noticed before ...
This whole condition should be covered by a single call to
const_ok_for_op. It already calls const_ok_for_arm internally.
Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
2011-06-06 13:33 ` Andrew Stubbs
@ 2011-06-06 14:26 ` Dmitry Plotnikov
2011-06-06 14:31 ` Andrew Stubbs
0 siblings, 1 reply; 16+ messages in thread
From: Dmitry Plotnikov @ 2011-06-06 14:26 UTC (permalink / raw)
To: Andrew Stubbs; +Cc: Ramana Radhakrishnan, gcc-patches, patches, dm
On 06/06/2011 05:33 PM, Andrew Stubbs wrote:
> On 06/06/11 14:26, Dmitry Plotnikov wrote:
>> if (const_ok_for_arm (INTVAL (x))
>> - || const_ok_for_arm (~INTVAL (x)))
>> + || const_ok_for_arm (~INTVAL (x))
>> + || (TARGET_THUMB2&& outer == PLUS
>> + && (const_ok_for_op (INTVAL (x), outer))))
>
> Sorry, I should have noticed before ...
>
> This whole condition should be covered by a single call to
> const_ok_for_op. It already calls const_ok_for_arm internally.
>
This condition is not covered by const_ok_for_op, because "outer" can be
equal to other rtx codes, which are not covered in const_ok_for_op like
IF_THEN_ELSE or MULT (I have several ICEs with these codes). I don't
know how to fix this correctly - should I add all codes to
const_ok_for_op or maybe just replace default alternative from
gcc_unreachable() to "return 0;" ?
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
2011-06-06 14:26 ` Dmitry Plotnikov
@ 2011-06-06 14:31 ` Andrew Stubbs
2011-06-15 13:12 ` Richard Earnshaw
0 siblings, 1 reply; 16+ messages in thread
From: Andrew Stubbs @ 2011-06-06 14:31 UTC (permalink / raw)
To: Dmitry Plotnikov; +Cc: Ramana Radhakrishnan, gcc-patches, patches, dm
On 06/06/11 15:26, Dmitry Plotnikov wrote:
> On 06/06/2011 05:33 PM, Andrew Stubbs wrote:
>> On 06/06/11 14:26, Dmitry Plotnikov wrote:
>>> if (const_ok_for_arm (INTVAL (x))
>>> - || const_ok_for_arm (~INTVAL (x)))
>>> + || const_ok_for_arm (~INTVAL (x))
>>> + || (TARGET_THUMB2&& outer == PLUS
>>> + && (const_ok_for_op (INTVAL (x), outer))))
>>
>> Sorry, I should have noticed before ...
>>
>> This whole condition should be covered by a single call to
>> const_ok_for_op. It already calls const_ok_for_arm internally.
>>
> This condition is not covered by const_ok_for_op, because "outer" can be
> equal to other rtx codes, which are not covered in const_ok_for_op like
> IF_THEN_ELSE or MULT (I have several ICEs with these codes). I don't
> know how to fix this correctly - should I add all codes to
> const_ok_for_op or maybe just replace default alternative from
> gcc_unreachable() to "return 0;" ?
That sounds reasonable to me - I mean, the constant is indeed not ok for
those operations.
... but I'm not a maintainer ....
Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
2011-06-06 14:31 ` Andrew Stubbs
@ 2011-06-15 13:12 ` Richard Earnshaw
0 siblings, 0 replies; 16+ messages in thread
From: Richard Earnshaw @ 2011-06-15 13:12 UTC (permalink / raw)
To: Andrew Stubbs
Cc: Dmitry Plotnikov, Ramana Radhakrishnan, gcc-patches, patches, dm
On 06/06/11 15:31, Andrew Stubbs wrote:
> On 06/06/11 15:26, Dmitry Plotnikov wrote:
>> On 06/06/2011 05:33 PM, Andrew Stubbs wrote:
>>> On 06/06/11 14:26, Dmitry Plotnikov wrote:
>>>> if (const_ok_for_arm (INTVAL (x))
>>>> - || const_ok_for_arm (~INTVAL (x)))
>>>> + || const_ok_for_arm (~INTVAL (x))
>>>> + || (TARGET_THUMB2&& outer == PLUS
>>>> + && (const_ok_for_op (INTVAL (x), outer))))
>>>
>>> Sorry, I should have noticed before ...
>>>
>>> This whole condition should be covered by a single call to
>>> const_ok_for_op. It already calls const_ok_for_arm internally.
>>>
>> This condition is not covered by const_ok_for_op, because "outer" can be
>> equal to other rtx codes, which are not covered in const_ok_for_op like
>> IF_THEN_ELSE or MULT (I have several ICEs with these codes). I don't
>> know how to fix this correctly - should I add all codes to
>> const_ok_for_op or maybe just replace default alternative from
>> gcc_unreachable() to "return 0;" ?
>
> That sounds reasonable to me - I mean, the constant is indeed not ok for
> those operations.
>
> ... but I'm not a maintainer ....
>
> Andrew
>
>
const_ok_for_op has grown beyond it's original intended use. A quick
look at it shows that a change along the way you describe would first
require fixing the assumption that if the constant matches
const_ok_for_arm in an unmodified form that it is OK universally. That
might be a good idea anyway, I'm not entirely sure that that's correct
for all possible use cases these days.
It's also likely that it needs extending to handle some of these cases
you describe. Having an outer of IF_THEN_ELSE most likely means this is
used in the case
(set reg if_then_else(cond) (op_or_const) (const))
for which the rules would be the same as if const were used directly in
a SET.
But it's also equally likely that we don't calculate the costs of
IF_THEN_ELSE very accurately anyway.
R.
R.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
2011-06-02 10:37 ` Ramana Radhakrishnan
@ 2011-06-16 9:41 ` Stubbs, Andrew
2011-08-26 12:04 ` Andrew Stubbs
0 siblings, 1 reply; 16+ messages in thread
From: Stubbs, Andrew @ 2011-06-16 9:41 UTC (permalink / raw)
To: Ramana Radhakrishnan; +Cc: gcc-patches, patches
On 02/06/11 11:36, Ramana Radhakrishnan wrote:
> On 2 June 2011 10:03, Andrew Stubbs<ams@codesourcery.com> wrote:
>> On 02/06/11 09:23, Ramana Radhakrishnan wrote:
>>>
>>> Please remove the alternatives in the subsi3 pattern since that is just
>>> unnecessary. Please make the constraints internal only.
>>
>> Is this better?
>>
>
> OK.
I've not yet committed this patch because my final testing revealed an
unexpected bootstrap failure. I'm still investigating.
I'll commit or post a replacement soon ... ish.
Andrew
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
2011-06-16 9:41 ` Stubbs, Andrew
@ 2011-08-26 12:04 ` Andrew Stubbs
0 siblings, 0 replies; 16+ messages in thread
From: Andrew Stubbs @ 2011-08-26 12:04 UTC (permalink / raw)
To: Stubbs, Andrew; +Cc: Ramana Radhakrishnan, gcc-patches, patches
[-- Attachment #1: Type: text/plain, Size: 502 bytes --]
On 16/06/11 10:13, Stubbs, Andrew wrote:
> On 02/06/11 11:36, Ramana Radhakrishnan wrote:
>> OK.
>
> I've not yet committed this patch because my final testing revealed an
> unexpected bootstrap failure. I'm still investigating.
>
> I'll commit or post a replacement soon ... ish.
Ok, it wasn't very soon, but I've now committed this patch.
In fact the bug was in my thumb2 replicated constants patch, so this one
is committed unchanged.
Actual patch updated for current baseline attached.
Andrew
[-- Attachment #2: addwsubw.patch --]
[-- Type: text/x-patch, Size: 4768 bytes --]
2011-08-26 Andrew Stubbs <ams@codesourcery.com>
gcc/
* config/arm/arm-protos.h (const_ok_for_op): Add prototype.
* config/arm/arm.c (const_ok_for_op): Add support for addw/subw.
Remove prototype. Remove static function type.
* config/arm/arm.md (*arm_addsi3): Add addw/subw support.
Add arch attribute.
* config/arm/constraints.md (Pj, PJ): New constraints.
--- a/gcc/config/arm/arm-protos.h
+++ b/gcc/config/arm/arm-protos.h
@@ -47,6 +47,7 @@ extern bool arm_vector_mode_supported_p (enum machine_mode);
extern bool arm_small_register_classes_for_mode_p (enum machine_mode);
extern int arm_hard_regno_mode_ok (unsigned int, enum machine_mode);
extern int const_ok_for_arm (HOST_WIDE_INT);
+extern int const_ok_for_op (HOST_WIDE_INT, enum rtx_code);
extern int arm_split_constant (RTX_CODE, enum machine_mode, rtx,
HOST_WIDE_INT, rtx, rtx, int);
extern RTX_CODE arm_canonicalize_comparison (RTX_CODE, rtx *, rtx *);
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -82,7 +82,6 @@ inline static int thumb1_index_register_rtx_p (rtx, int);
static bool arm_legitimate_address_p (enum machine_mode, rtx, bool);
static int thumb_far_jump_used_p (void);
static bool thumb_force_lr_save (void);
-static int const_ok_for_op (HOST_WIDE_INT, enum rtx_code);
static rtx emit_sfm (int, int);
static unsigned arm_size_return_regs (void);
static bool arm_assemble_integer (rtx, unsigned int, int);
@@ -2375,7 +2374,7 @@ const_ok_for_arm (HOST_WIDE_INT i)
}
/* Return true if I is a valid constant for the operation CODE. */
-static int
+int
const_ok_for_op (HOST_WIDE_INT i, enum rtx_code code)
{
if (const_ok_for_arm (i))
@@ -2392,6 +2391,13 @@ const_ok_for_op (HOST_WIDE_INT i, enum rtx_code code)
return const_ok_for_arm (ARM_SIGN_EXTEND (~i));
case PLUS:
+ /* See if we can use addw or subw. */
+ if (TARGET_THUMB2
+ && ((i & 0xfffff000) == 0
+ || ((-i) & 0xfffff000) == 0))
+ return 1;
+ /* else fall through. */
+
case COMPARE:
case EQ:
case NE:
--- a/gcc/config/arm/arm.md
+++ b/gcc/config/arm/arm.md
@@ -708,21 +708,24 @@
;; (plus (reg rN) (reg sp)) into (reg rN). In this case reload will
;; put the duplicated register first, and not try the commutative version.
(define_insn_and_split "*arm_addsi3"
- [(set (match_operand:SI 0 "s_register_operand" "=r, k,r,r, k,r")
- (plus:SI (match_operand:SI 1 "s_register_operand" "%rk,k,r,rk,k,rk")
- (match_operand:SI 2 "reg_or_int_operand" "rI,rI,k,L, L,?n")))]
+ [(set (match_operand:SI 0 "s_register_operand" "=r, k,r,r, k, r, k,r, k, r")
+ (plus:SI (match_operand:SI 1 "s_register_operand" "%rk,k,r,rk,k, rk,k,rk,k, rk")
+ (match_operand:SI 2 "reg_or_int_operand" "rI,rI,k,Pj,Pj,L, L,PJ,PJ,?n")))]
"TARGET_32BIT"
"@
add%?\\t%0, %1, %2
add%?\\t%0, %1, %2
add%?\\t%0, %2, %1
+ addw%?\\t%0, %1, %2
+ addw%?\\t%0, %1, %2
sub%?\\t%0, %1, #%n2
sub%?\\t%0, %1, #%n2
+ subw%?\\t%0, %1, #%n2
+ subw%?\\t%0, %1, #%n2
#"
"TARGET_32BIT
&& GET_CODE (operands[2]) == CONST_INT
- && !(const_ok_for_arm (INTVAL (operands[2]))
- || const_ok_for_arm (-INTVAL (operands[2])))
+ && !const_ok_for_op (INTVAL (operands[2]), PLUS)
&& (reload_completed || !arm_eliminable_register (operands[1]))"
[(clobber (const_int 0))]
"
@@ -731,8 +734,9 @@
operands[1], 0);
DONE;
"
- [(set_attr "length" "4,4,4,4,4,16")
- (set_attr "predicable" "yes")]
+ [(set_attr "length" "4,4,4,4,4,4,4,4,4,16")
+ (set_attr "predicable" "yes")
+ (set_attr "arch" "*,*,*,t2,t2,*,*,t2,t2,*")]
)
(define_insn_and_split "*thumb1_addsi3"
--- a/gcc/config/arm/constraints.md
+++ b/gcc/config/arm/constraints.md
@@ -31,7 +31,7 @@
;; The following multi-letter normal constraints have been used:
;; in ARM/Thumb-2 state: Da, Db, Dc, Dn, Dl, DL, Dv, Dy, Di, Dz
;; in Thumb-1 state: Pa, Pb, Pc, Pd
-;; in Thumb-2 state: Ps, Pt, Pu, Pv, Pw, Px, Py
+;; in Thumb-2 state: Pj, PJ, Ps, Pt, Pu, Pv, Pw, Px, Py
;; The following memory constraints have been used:
;; in ARM/Thumb-2 state: Q, Ut, Uv, Uy, Un, Um, Us
@@ -75,6 +75,18 @@
(and (match_code "const_int")
(match_test "(ival & 0xffff0000) == 0")))))
+(define_constraint "Pj"
+ "@internal A 12-bit constant suitable for an ADDW or SUBW instruction. (Thumb-2)"
+ (and (match_code "const_int")
+ (and (match_test "TARGET_THUMB2")
+ (match_test "(ival & 0xfffff000) == 0"))))
+
+(define_constraint "PJ"
+ "@internal A constant that satisfies the Pj constrant if negated."
+ (and (match_code "const_int")
+ (and (match_test "TARGET_THUMB2")
+ (match_test "((-ival) & 0xfffff000) == 0"))))
+
(define_register_constraint "k" "STACK_REG"
"@internal The stack register.")
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2011-08-26 9:59 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-20 15:36 [PATCH][ARM] Add support for ADDW and SUBW instructions Andrew Stubbs
2011-04-20 15:56 ` Andrew Stubbs
2011-05-18 15:47 ` Andrew Stubbs
2011-06-02 8:07 ` Andrew Stubbs
2011-06-02 8:24 ` Ramana Radhakrishnan
2011-06-02 9:03 ` Andrew Stubbs
2011-06-02 10:37 ` Ramana Radhakrishnan
2011-06-16 9:41 ` Stubbs, Andrew
2011-08-26 12:04 ` Andrew Stubbs
2011-06-06 12:15 ` Dmitry Plotnikov
2011-06-06 12:41 ` Andrew Stubbs
2011-06-06 13:26 ` Dmitry Plotnikov
2011-06-06 13:33 ` Andrew Stubbs
2011-06-06 14:26 ` Dmitry Plotnikov
2011-06-06 14:31 ` Andrew Stubbs
2011-06-15 13:12 ` Richard Earnshaw
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).