Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
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.
Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
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 2011-06-06 Dmitry PLotnikov dplotni...@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,
Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
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? 2011-06-06 Dmitry PLotnikov dplotni...@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,
Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
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
Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
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; ?
Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
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
Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
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
Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
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
Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
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
2011-06-02 Andrew Stubbs a...@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 0xf000) == 0
+ || ((-i) 0xf000) == 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 0x) == 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 0xf000) == 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) 0xf000) == 0
+
(define_register_constraint k STACK_REG
@internal The stack register.)
Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
On 2 June 2011 10:03, Andrew Stubbs a...@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
Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
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
[PATCH][ARM] Add support for ADDW and SUBW instructions
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
2011-04-20 Andrew Stubbs a...@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 0xf000) == 0
+ || ((-i) 0xf000) == 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
Re: [PATCH][ARM] Add support for ADDW and SUBW instructions
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
