* [PATCH][MSP430] Add support for post increment addressing
@ 2019-10-07 20:28 Jozef Lawrynowicz
2019-10-13 16:29 ` Jeff Law
0 siblings, 1 reply; 4+ messages in thread
From: Jozef Lawrynowicz @ 2019-10-07 20:28 UTC (permalink / raw)
To: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 217 bytes --]
MSP430 supports post increment addressing for the source operand only. The
attached patch enables post increment addressing for MSP430 in GCC.
Regtested on trunk for the small and large memory models.
Ok for trunk?
[-- Attachment #2: 0001-MSP430-Implement-post-increment-addressing.patch --]
[-- Type: text/x-patch, Size: 33204 bytes --]
From d75e48ba434d7bab141c09d442793b2cb26afa69 Mon Sep 17 00:00:00 2001
From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
Date: Mon, 16 Sep 2019 16:34:51 +0100
Subject: [PATCH] MSP430: Implement post increment addressing
gcc/ChangeLog:
2019-10-07 Jozef Lawrynowicz <jozef.l@mittosystems.com>
* config/msp430/constraints.md: Allow post_inc operand for "Ya"
constraint.
* config/msp430/msp430.c (msp430_legitimate_address_p): Handle
POST_INC.
(msp430_subreg): Likewise.
(msp430_split_addsi): Likewise.
(msp430_print_operand_addr): Likewise.
* config/msp430/msp430.h (HAVE_POST_INCREMENT): Define.
(USE_STORE_POST_INCREMENT): Define.
* config/msp430/msp430.md: Use the msp430_general_dst_operand or
msp430_general_dst_nonv_operand predicates for the lvalues insns.
* config/msp430/predicates.md (msp430_nonpostinc_operand): New.
(msp430_general_dst_operand): New.
(msp430_general_dst_nonv_operand): New.
(msp430_nonsubreg_operand): Remove.
(msp430_nonsubreg_dst_operand): New.
(msp430_nonsubreg_or_imm_operand): Allow reg or mem operands in place
of defunct msp430_nonsubreg_operand.
(msp430_nonsubregnonpostinc_or_imm_operand): New.
---
gcc/config/msp430/constraints.md | 1 +
gcc/config/msp430/msp430.c | 32 +++++-
gcc/config/msp430/msp430.h | 12 ++
gcc/config/msp430/msp430.md | 191 ++++++++++++++++---------------
gcc/config/msp430/predicates.md | 46 +++++++-
5 files changed, 183 insertions(+), 99 deletions(-)
diff --git a/gcc/config/msp430/constraints.md b/gcc/config/msp430/constraints.md
index 4422b2b6454..d01bcf9a242 100644
--- a/gcc/config/msp430/constraints.md
+++ b/gcc/config/msp430/constraints.md
@@ -60,6 +60,7 @@
(match_code "reg" "00")
(match_test ("CONST_INT_P (XEXP (XEXP (op, 0), 1))")))
(match_test "CONSTANT_P (XEXP (op, 0))")
+ (match_code "post_inc" "0")
)))
(define_constraint "Yl"
diff --git a/gcc/config/msp430/msp430.c b/gcc/config/msp430/msp430.c
index ae763faada3..31029395c3d 100644
--- a/gcc/config/msp430/msp430.c
+++ b/gcc/config/msp430/msp430.c
@@ -942,12 +942,17 @@ msp430_legitimate_address_p (machine_mode mode ATTRIBUTE_UNUSED,
return false;
case PLUS:
+ case POST_INC:
if (REG_P (XEXP (x, 0)))
{
if (GET_MODE (x) != GET_MODE (XEXP (x, 0)))
return false;
if (!reg_ok_for_addr (XEXP (x, 0), strict))
return false;
+ if (GET_CODE (x) == POST_INC)
+ /* At this point, if the original rtx was a post_inc, we don't have
+ anything further to check. */
+ return true;
switch (GET_CODE (XEXP (x, 1)))
{
case CONST:
@@ -2810,6 +2815,7 @@ rtx
msp430_subreg (machine_mode mode, rtx r, machine_mode omode, int byte)
{
rtx rv;
+ gcc_assert (mode == HImode);
if (GET_CODE (r) == SUBREG
&& SUBREG_BYTE (r) == 0)
@@ -2826,7 +2832,15 @@ msp430_subreg (machine_mode mode, rtx r, machine_mode omode, int byte)
rv = simplify_gen_subreg (mode, ireg, imode, byte);
}
else if (GET_CODE (r) == MEM)
- rv = adjust_address (r, mode, byte);
+ {
+ /* When byte == 2, we can be certain that we were already called with an
+ identical rtx with byte == 0. So we don't need to do anything to
+ get a 2 byte offset of a (mem (post_inc)) rtx, since the address has
+ already been offset by the post_inc itself. */
+ if (GET_CODE (XEXP (r, 0)) == POST_INC && byte == 2)
+ byte = 0;
+ rv = adjust_address (r, mode, byte);
+ }
else if (GET_CODE (r) == SYMBOL_REF
&& (byte == 0 || byte == 2)
&& mode == HImode)
@@ -2861,6 +2875,18 @@ msp430_split_addsi (rtx *operands)
if (GET_CODE (operands[5]) == CONST_INT)
operands[9] = GEN_INT (INTVAL (operands[5]) & 0xffff);
+ /* Handle post_inc, for example:
+ (set (reg:SI)
+ (plus:SI (reg:SI)
+ (mem:SI (post_inc:PSI (reg:PSI))))). */
+ else if (MEM_P (operands[5]) && GET_CODE (XEXP (operands[5], 0)) == POST_INC)
+ {
+ /* Strip out the post_inc from (mem (post_inc (reg))). */
+ operands[9] = XEXP (XEXP (operands[5], 0), 0);
+ operands[9] = gen_rtx_MEM (HImode, operands[9]);
+ /* Then zero extend as normal. */
+ operands[9] = gen_rtx_ZERO_EXTEND (SImode, operands[9]);
+ }
else
operands[9] = gen_rtx_ZERO_EXTEND (SImode, operands[5]);
return 0;
@@ -3205,6 +3231,10 @@ msp430_print_operand_addr (FILE * file, machine_mode /*mode*/, rtx addr)
fprintf (file, "@");
break;
+ case POST_INC:
+ fprintf (file, "@%s+", reg_names[REGNO (XEXP (addr, 0))]);
+ return;
+
case CONST:
case CONST_INT:
case SYMBOL_REF:
diff --git a/gcc/config/msp430/msp430.h b/gcc/config/msp430/msp430.h
index f885de2bb2f..73afe2e2d16 100644
--- a/gcc/config/msp430/msp430.h
+++ b/gcc/config/msp430/msp430.h
@@ -478,6 +478,18 @@ typedef struct
#define ACCUMULATE_OUTGOING_ARGS 1
+#define HAVE_POST_INCREMENT 1
+
+/* This (unsurprisingly) improves code size in the vast majority of cases, we
+ want to prevent any instructions using a "store post increment" from being
+ generated. These will have to later be reloaded since msp430 does not
+ support post inc for the destination operand. */
+#define USE_STORE_POST_INCREMENT(MODE) 0
+
+/* Many other targets set USE_LOAD_POST_INCREMENT to 0. For msp430-elf
+ the benefit of disabling it is not clear. When looking at code size, on
+ average, there is a slight advantage to leaving it enabled. */
+
#undef ASM_DECLARE_FUNCTION_NAME
#define ASM_DECLARE_FUNCTION_NAME(FILE, NAME, DECL) \
msp430_start_function ((FILE), (NAME), (DECL))
diff --git a/gcc/config/msp430/msp430.md b/gcc/config/msp430/msp430.md
index ebd9c85fbb6..a533efa1656 100644
--- a/gcc/config/msp430/msp430.md
+++ b/gcc/config/msp430/msp430.md
@@ -183,14 +183,14 @@
)
(define_insn "movqi_topbyte"
- [(set (match_operand:QI 0 "msp430_nonimmediate_operand" "=r")
+ [(set (match_operand:QI 0 "msp430_general_dst_operand" "=r")
(subreg:QI (match_operand:PSI 1 "msp430_general_operand" "r") 2))]
"msp430x"
"PUSHM.A\t#1,%1 { POPM.W\t#1,%0 { POPM.W\t#1,%0"
)
(define_insn "movqi"
- [(set (match_operand:QI 0 "msp430_nonimmediate_operand" "=rYsYx,rm")
+ [(set (match_operand:QI 0 "msp430_general_dst_operand" "=rYsYx,rm")
(match_operand:QI 1 "msp430_general_operand" "riYsYx,rmi"))]
""
"@
@@ -199,7 +199,7 @@
)
(define_insn "movhi"
- [(set (match_operand:HI 0 "msp430_nonimmediate_operand" "=r,rYsYx,rm")
+ [(set (match_operand:HI 0 "msp430_general_dst_operand" "=r,rYsYx,rm")
(match_operand:HI 1 "msp430_general_operand" "N,riYsYx,rmi"))]
""
"@
@@ -209,41 +209,41 @@
)
(define_expand "movsi"
- [(set (match_operand:SI 0 "nonimmediate_operand")
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand")
(match_operand:SI 1 "general_operand"))]
""
""
)
(define_insn_and_split "movsi_s"
- [(set (match_operand:SI 0 "nonimmediate_operand" "=rm")
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand" "=rm")
(subreg:SI (match_operand:PSI 1 "msp430_symbol_operand" "i") 0))]
""
""
"reload_completed"
- [(set (match_operand:HI 2 "nonimmediate_operand")
+ [(set (match_operand:HI 2 "msp430_general_dst_nonv_operand")
(match_operand:HI 4 "general_operand"))
- (set (match_operand:HI 3 "nonimmediate_operand")
+ (set (match_operand:HI 3 "msp430_general_dst_nonv_operand")
(match_operand:HI 5 "general_operand"))]
"msp430_split_movsi (operands);"
)
(define_insn_and_split "movsi_x"
- [(set (match_operand:SI 0 "nonimmediate_operand" "=rm")
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand" "=rm")
(match_operand:SI 1 "general_operand" "rmi"))]
""
"#"
"reload_completed"
- [(set (match_operand:HI 2 "nonimmediate_operand")
+ [(set (match_operand:HI 2 "msp430_general_dst_nonv_operand")
(match_operand:HI 4 "general_operand"))
- (set (match_operand:HI 3 "nonimmediate_operand")
+ (set (match_operand:HI 3 "msp430_general_dst_nonv_operand")
(match_operand:HI 5 "general_operand"))]
"msp430_split_movsi (operands);"
)
;; FIXME: Some MOVX.A cases can be done with MOVA, this is only a few of them.
(define_insn "movpsi"
- [(set (match_operand:PSI 0 "msp430_nonimmediate_operand" "=r,r,r,Ya,rm")
+ [(set (match_operand:PSI 0 "msp430_general_dst_operand" "=r,r,r,Ya,rm")
(match_operand:PSI 1 "msp430_general_operand" "N,O,riYa,r,rmi"))]
""
"@
@@ -279,8 +279,8 @@
;; Math
(define_insn "addpsi3"
- [(set (match_operand:PSI 0 "msp430_nonimmediate_operand" "=r,rm")
- (plus:PSI (match_operand:PSI 1 "msp430_nonimmediate_operand" "%0,0")
+ [(set (match_operand:PSI 0 "msp430_general_dst_operand" "=r,rm")
+ (plus:PSI (match_operand:PSI 1 "msp430_general_operand" "%0,0")
(match_operand:PSI 2 "msp430_general_operand" "rLs,rmi")))]
""
"@
@@ -289,8 +289,8 @@
)
(define_insn "addqi3"
- [(set (match_operand:QI 0 "msp430_nonimmediate_operand" "=rYsYx,rm")
- (plus:QI (match_operand:QI 1 "msp430_nonimmediate_operand" "%0,0")
+ [(set (match_operand:QI 0 "msp430_general_dst_operand" "=rYsYx,rm")
+ (plus:QI (match_operand:QI 1 "msp430_general_operand" "%0,0")
(match_operand:QI 2 "msp430_general_operand" "riYsYx,rmi")))]
""
"@
@@ -299,8 +299,8 @@
)
(define_insn "addhi3"
- [(set (match_operand:HI 0 "msp430_nonimmediate_operand" "=rYsYx,rm")
- (plus:HI (match_operand:HI 1 "msp430_nonimmediate_operand" "%0,0")
+ [(set (match_operand:HI 0 "msp430_general_dst_operand" "=rYsYx,rm")
+ (plus:HI (match_operand:HI 1 "msp430_general_operand" "%0,0")
(match_operand:HI 2 "msp430_general_operand" "riYsYx,rmi")))]
""
"@
@@ -321,8 +321,8 @@
)
(define_insn "addsi3"
- [(set (match_operand:SI 0 "nonimmediate_operand" "=&rYsYx,rm")
- (plus:SI (match_operand:SI 1 "nonimmediate_operand" "%0,0")
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand" "=&rYsYx,rm")
+ (plus:SI (match_operand:SI 1 "general_operand" "%0,0")
(match_operand:SI 2 "general_operand" "rYsYxi,mi")))]
""
"@
@@ -357,13 +357,16 @@
; increased register pressure. Or possibly reload does not handle ADD patterns
; that are not single_set() very well.
+; match_operand 3 is likely to be the same as op2 most of the time - except
+; when op2 is a post_inc and we have stripped the post_inc from match_operand 3
+
(define_insn "addhi3_cy"
- [(set (match_operand:HI 0 "msp430_nonimmediate_operand" "=rYsYx,rm")
- (plus:HI (match_operand:HI 1 "msp430_nonimmediate_operand" "%0,0")
+ [(set (match_operand:HI 0 "msp430_general_dst_operand" "=rYsYx,rm")
+ (plus:HI (match_operand:HI 1 "msp430_general_operand" "%0,0")
(match_operand:HI 2 "msp430_nonimmediate_operand" "rYsYxi,rm")))
(set (reg:BI CARRY)
(truncate:BI (lshiftrt:SI (plus:SI (zero_extend:SI (match_dup 1))
- (zero_extend:SI (match_dup 2)))
+ (zero_extend:SI (match_operand:HI 3 "msp430_nonimmediate_operand" "rYsYxi,rm")))
(const_int 16))))
]
""
@@ -373,8 +376,8 @@
)
(define_insn "addhi3_cy_i"
- [(set (match_operand:HI 0 "nonimmediate_operand" "=r,rm")
- (plus:HI (match_operand:HI 1 "nonimmediate_operand" "%0,0")
+ [(set (match_operand:HI 0 "msp430_general_dst_nonv_operand" "=r,rm")
+ (plus:HI (match_operand:HI 1 "general_operand" "%0,0")
(match_operand:HI 2 "immediate_operand" "i,i")))
(set (reg:BI CARRY)
(truncate:BI (lshiftrt:SI (plus:SI (zero_extend:SI (match_dup 1))
@@ -389,8 +392,8 @@
; Version of addhi that adds the carry, for SImode adds.
(define_insn "addchi4_cy"
- [(set (match_operand:HI 0 "msp430_nonimmediate_operand" "=rYsYx,rm")
- (plus:HI (plus:HI (match_operand:HI 1 "msp430_nonimmediate_operand" "%0,0")
+ [(set (match_operand:HI 0 "msp430_general_dst_operand" "=rYsYx,rm")
+ (plus:HI (plus:HI (match_operand:HI 1 "msp430_general_operand" "%0,0")
(match_operand:HI 2 "msp430_general_operand" "riYsYx,rmi"))
(zero_extend:HI (reg:BI CARRY))))
]
@@ -403,13 +406,15 @@
; Split an SImode add into two HImode adds, keeping track of the carry
; so that gcc knows when it can and can't optimize away the two
; halves.
+; We use the ugly predicate "msp430_nonsubregnonpostinc_or_imm_operand" to
+; enforce the position of a post_inc into op2 if present
(define_split
- [(set (match_operand:SI 0 "msp430_nonsubreg_operand")
- (plus:SI (match_operand:SI 1 "msp430_nonsubreg_operand")
+ [(set (match_operand:SI 0 "msp430_nonsubreg_dst_operand")
+ (plus:SI (match_operand:SI 1 "msp430_nonsubregnonpostinc_or_imm_operand")
(match_operand:SI 2 "msp430_nonsubreg_or_imm_operand")))
]
""
- [(parallel [(set (match_operand:HI 3 "nonimmediate_operand" "=&rm")
+ [(parallel [(set (match_operand:HI 3 "msp430_general_dst_nonv_operand" "=&rm")
(plus:HI (match_dup 4)
(match_dup 5)))
(set (reg:BI CARRY)
@@ -417,7 +422,7 @@
(match_dup 9))
(const_int 16))))
])
- (set (match_operand:HI 6 "nonimmediate_operand" "=&rm")
+ (set (match_operand:HI 6 "msp430_general_dst_nonv_operand" "=&rm")
(plus:HI (plus:HI (match_dup 7)
(match_dup 8))
(zero_extend:HI (reg:BI CARRY))))
@@ -431,9 +436,9 @@
;; Alternatives 2 and 3 are to handle cases generated by reload.
(define_insn "subpsi3"
- [(set (match_operand:PSI 0 "nonimmediate_operand" "=r, rm, &?r, ?&r")
- (minus:PSI (match_operand:PSI 1 "general_operand" "0, 0, !r, !i")
- (match_operand:PSI 2 "general_operand" "rLs, rmi, rmi, r")))]
+ [(set (match_operand:PSI 0 "msp430_general_dst_nonv_operand" "=r, rm, &?r, ?&r")
+ (minus:PSI (match_operand:PSI 1 "general_operand" "0, 0, !r, !i")
+ (match_operand:PSI 2 "general_operand" "rLs, rmi, rmi, r")))]
""
"@
SUBA\t%2, %0
@@ -444,7 +449,7 @@
;; Alternatives 2 and 3 are to handle cases generated by reload.
(define_insn "subqi3"
- [(set (match_operand:QI 0 "nonimmediate_operand" "=rYsYx, rm, &?r, ?&r")
+ [(set (match_operand:QI 0 "msp430_general_dst_nonv_operand" "=rYsYx, rm, &?r, ?&r")
(minus:QI (match_operand:QI 1 "general_operand" "0, 0, !r, !i")
(match_operand:QI 2 "general_operand" " riYsYx, rmi, rmi, r")))]
""
@@ -457,7 +462,7 @@
;; Alternatives 2 and 3 are to handle cases generated by reload.
(define_insn "subhi3"
- [(set (match_operand:HI 0 "nonimmediate_operand" "=rYsYx, rm, &?r, ?&r")
+ [(set (match_operand:HI 0 "msp430_general_dst_nonv_operand" "=rYsYx, rm, &?r, ?&r")
(minus:HI (match_operand:HI 1 "general_operand" "0, 0, !r, !i")
(match_operand:HI 2 "general_operand" " riYsYx, rmi, rmi, r")))]
""
@@ -469,8 +474,8 @@
)
(define_insn "subsi3"
- [(set (match_operand:SI 0 "nonimmediate_operand" "=&rYsYx,m")
- (minus:SI (match_operand:SI 1 "nonimmediate_operand" "0,0")
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand" "=&rYsYx,m")
+ (minus:SI (match_operand:SI 1 "general_operand" "0,0")
(match_operand:SI 2 "general_operand" "riYsYx,mi")))]
""
"@
@@ -479,7 +484,7 @@
)
(define_insn "*bic<mode>_cg"
- [(set (match_operand:QHI 0 "msp430_nonimmediate_operand" "=rYs,m")
+ [(set (match_operand:QHI 0 "msp430_general_dst_operand" "=rYs,m")
(and:QHI (match_operand:QHI 1 "msp430_general_operand" "0,0")
(match_operand 2 "msp430_inv_constgen_operator" "n,n")))]
""
@@ -489,9 +494,9 @@
)
(define_insn "bic<mode>3"
- [(set (match_operand:QHI 0 "msp430_nonimmediate_operand" "=rYsYx,rm")
+ [(set (match_operand:QHI 0 "msp430_general_dst_operand" "=rYsYx,rm")
(and:QHI (not:QHI (match_operand:QHI 1 "msp430_general_operand" "rYsYx,rmn"))
- (match_operand:QHI 2 "msp430_nonimmediate_operand" "0,0")))]
+ (match_operand:QHI 2 "msp430_general_operand" "0,0")))]
""
"@
BIC%x0%b0\t%1, %0
@@ -499,8 +504,8 @@
)
(define_insn "and<mode>3"
- [(set (match_operand:QHI 0 "msp430_nonimmediate_operand" "=r,rYsYx,rm")
- (and:QHI (match_operand:QHI 1 "msp430_nonimmediate_operand" "%0,0,0")
+ [(set (match_operand:QHI 0 "msp430_general_dst_operand" "=r,rYsYx,rm")
+ (and:QHI (match_operand:QHI 1 "msp430_general_operand" "%0,0,0")
(match_operand:QHI 2 "msp430_general_operand" "N,riYsYx,rmi")))]
""
"@
@@ -510,8 +515,8 @@
)
(define_insn "ior<mode>3"
- [(set (match_operand:QHI 0 "msp430_nonimmediate_operand" "=rYsYx,rm")
- (ior:QHI (match_operand:QHI 1 "msp430_nonimmediate_operand" "%0,0")
+ [(set (match_operand:QHI 0 "msp430_general_dst_operand" "=rYsYx,rm")
+ (ior:QHI (match_operand:QHI 1 "msp430_general_operand" "%0,0")
(match_operand:QHI 2 "msp430_general_operand" "riYsYx,rmi")))]
""
"@
@@ -520,8 +525,8 @@
)
(define_insn "xor<mode>3"
- [(set (match_operand:QHI 0 "msp430_nonimmediate_operand" "=rYsYx,rm")
- (xor:QHI (match_operand:QHI 1 "msp430_nonimmediate_operand" "%0,0")
+ [(set (match_operand:QHI 0 "msp430_general_dst_operand" "=rYsYx,rm")
+ (xor:QHI (match_operand:QHI 1 "msp430_general_operand" "%0,0")
(match_operand:QHI 2 "msp430_general_operand" "riYsYx,rmi")))]
""
"@
@@ -531,8 +536,8 @@
;; Macro : XOR #~0, %0
(define_insn "one_cmpl<mode>2"
- [(set (match_operand:QHI 0 "msp430_nonimmediate_operand" "=rYs,m")
- (not:QHI (match_operand:QHI 1 "msp430_nonimmediate_operand" "0,0")))]
+ [(set (match_operand:QHI 0 "msp430_general_dst_operand" "=rYs,m")
+ (not:QHI (match_operand:QHI 1 "msp430_general_operand" "0,0")))]
""
"@
INV%x0%b0\t%0
@@ -540,8 +545,8 @@
)
(define_insn "extendqihi2"
- [(set (match_operand:HI 0 "msp430_nonimmediate_operand" "=rYs,m")
- (sign_extend:HI (match_operand:QI 1 "msp430_nonimmediate_operand" "0,0")))]
+ [(set (match_operand:HI 0 "msp430_general_dst_operand" "=rYs,m")
+ (sign_extend:HI (match_operand:QI 1 "msp430_general_operand" "0,0")))]
""
"@
SXT%X0\t%0
@@ -549,8 +554,8 @@
)
(define_insn "zero_extendqihi2"
- [(set (match_operand:HI 0 "msp430_nonimmediate_operand" "=rYs,r,r,m")
- (zero_extend:HI (match_operand:QI 1 "msp430_nonimmediate_operand" "0,rYs,m,0")))]
+ [(set (match_operand:HI 0 "msp430_general_dst_operand" "=rYs,r,r,m")
+ (zero_extend:HI (match_operand:QI 1 "msp430_general_operand" "0,rYs,m,0")))]
""
"@
AND\t#0xff, %0
@@ -571,8 +576,8 @@
)
(define_insn "zero_extendhipsi2"
- [(set (match_operand:PSI 0 "msp430_nonimmediate_operand" "=r,m")
- (zero_extend:PSI (match_operand:HI 1 "msp430_nonimmediate_operand" "rm,r")))]
+ [(set (match_operand:PSI 0 "msp430_general_dst_operand" "=r,m")
+ (zero_extend:PSI (match_operand:HI 1 "msp430_general_operand" "rm,r")))]
""
"@
MOVX\t%1, %0
@@ -580,22 +585,22 @@
)
(define_insn "truncpsihi2"
- [(set (match_operand:HI 0 "msp430_nonimmediate_operand" "=rm")
+ [(set (match_operand:HI 0 "msp430_general_dst_operand" "=rm")
(truncate:HI (match_operand:PSI 1 "register_operand" "r")))]
""
"MOVX\t%1, %0"
)
(define_insn "extendhisi2"
- [(set (match_operand:SI 0 "nonimmediate_operand" "=r")
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand" "=r")
(sign_extend:SI (match_operand:HI 1 "nonimmediate_operand" "r")))]
""
{ return msp430x_extendhisi (operands); }
)
(define_insn "extendhipsi2"
- [(set (match_operand:PSI 0 "nonimmediate_operand" "=r")
- (subreg:PSI (sign_extend:SI (match_operand:HI 1 "nonimmediate_operand" "0")) 0))]
+ [(set (match_operand:PSI 0 "msp430_general_dst_nonv_operand" "=r")
+ (subreg:PSI (sign_extend:SI (match_operand:HI 1 "general_operand" "0")) 0))]
"msp430x"
"RLAM.A #4, %0 { RRAM.A #4, %0"
)
@@ -606,15 +611,15 @@
;; paths.
(define_insn "zero_extendqisi2"
- [(set (match_operand:SI 0 "nonimmediate_operand" "=r")
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand" "=r")
(zero_extend:SI (match_operand:QI 1 "nonimmediate_operand" "rm")))]
""
"MOV%X1.B\t%1,%L0 { CLR\t%H0"
)
(define_insn "zero_extendhisi2"
- [(set (match_operand:SI 0 "nonimmediate_operand" "=rm,r")
- (zero_extend:SI (match_operand:HI 1 "nonimmediate_operand" "0,r")))]
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand" "=rm,r")
+ (zero_extend:SI (match_operand:HI 1 "general_operand" "0,r")))]
""
"@
MOV%X0.W\t#0,%H0
@@ -622,8 +627,8 @@
)
(define_insn "zero_extendhisipsi2"
- [(set (match_operand:PSI 0 "nonimmediate_operand" "=r,r")
- (subreg:PSI (zero_extend:SI (match_operand:HI 1 "nonimmediate_operand" "0,r")) 0))]
+ [(set (match_operand:PSI 0 "msp430_general_dst_nonv_operand" "=r,r")
+ (subreg:PSI (zero_extend:SI (match_operand:HI 1 "general_operand" "0,r")) 0))]
"msp430x"
"@
AND.W\t#-1,%0
@@ -631,16 +636,16 @@
)
(define_insn "extend_and_shift1_hipsi2"
- [(set (subreg:SI (match_operand:PSI 0 "nonimmediate_operand" "=r") 0)
- (ashift:SI (sign_extend:SI (match_operand:HI 1 "nonimmediate_operand" "0"))
+ [(set (subreg:SI (match_operand:PSI 0 "msp430_general_dst_nonv_operand" "=r") 0)
+ (ashift:SI (sign_extend:SI (match_operand:HI 1 "general_operand" "0"))
(const_int 1)))]
"msp430x"
"RLAM.A #4, %0 { RRAM.A #3, %0"
)
(define_insn "extend_and_shift2_hipsi2"
- [(set (subreg:SI (match_operand:PSI 0 "nonimmediate_operand" "=r") 0)
- (ashift:SI (sign_extend:SI (match_operand:HI 1 "nonimmediate_operand" "0"))
+ [(set (subreg:SI (match_operand:PSI 0 "msp430_general_dst_nonv_operand" "=r") 0)
+ (ashift:SI (sign_extend:SI (match_operand:HI 1 "general_operand" "0"))
(const_int 2)))]
"msp430x"
"RLAM.A #4, %0 { RRAM.A #2, %0"
@@ -739,7 +744,7 @@
;; signed A << C
(define_expand "ashlhi3"
- [(set (match_operand:HI 0 "nonimmediate_operand")
+ [(set (match_operand:HI 0 "msp430_general_dst_nonv_operand")
(ashift:HI (match_operand:HI 1 "general_operand")
(match_operand:HI 2 "general_operand")))]
""
@@ -766,7 +771,7 @@
)
(define_insn "slli_1"
- [(set (match_operand:HI 0 "nonimmediate_operand" "=rm")
+ [(set (match_operand:HI 0 "msp430_general_dst_nonv_operand" "=rm")
(ashift:HI (match_operand:HI 1 "general_operand" "0")
(const_int 1)))]
""
@@ -786,7 +791,7 @@
)
(define_insn "slll_1"
- [(set (match_operand:SI 0 "nonimmediate_operand" "=rm")
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand" "=rm")
(ashift:SI (match_operand:SI 1 "general_operand" "0")
(const_int 1)))]
""
@@ -794,7 +799,7 @@
)
(define_insn "slll_2"
- [(set (match_operand:SI 0 "nonimmediate_operand" "=rm")
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand" "=rm")
(ashift:SI (match_operand:SI 1 "general_operand" "0")
(const_int 2)))]
""
@@ -802,7 +807,7 @@
)
(define_expand "ashlsi3"
- [(set (match_operand:SI 0 "nonimmediate_operand")
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand")
(ashift:SI (match_operand:SI 1 "general_operand")
(match_operand:SI 2 "general_operand")))]
""
@@ -811,7 +816,7 @@
)
(define_expand "ashldi3"
- [(set (match_operand:DI 0 "nonimmediate_operand")
+ [(set (match_operand:DI 0 "msp430_general_dst_nonv_operand")
(ashift:DI (match_operand:DI 1 "general_operand")
(match_operand:DI 2 "general_operand")))]
""
@@ -827,7 +832,7 @@
;; signed A >> C
(define_expand "ashrhi3"
- [(set (match_operand:HI 0 "nonimmediate_operand")
+ [(set (match_operand:HI 0 "msp430_general_dst_nonv_operand")
(ashiftrt:HI (match_operand:HI 1 "general_operand")
(match_operand:HI 2 "general_operand")))]
""
@@ -851,7 +856,7 @@
)
(define_insn "srai_1"
- [(set (match_operand:HI 0 "msp430_nonimmediate_operand" "=rm")
+ [(set (match_operand:HI 0 "msp430_general_dst_operand" "=rm")
(ashiftrt:HI (match_operand:HI 1 "msp430_general_operand" "0")
(const_int 1)))]
""
@@ -887,7 +892,7 @@
)
(define_insn "sral_1"
- [(set (match_operand:SI 0 "nonimmediate_operand" "=rm")
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand" "=rm")
(ashiftrt:SI (match_operand:SI 1 "general_operand" "0")
(const_int 1)))]
""
@@ -895,7 +900,7 @@
)
(define_insn "sral_2"
- [(set (match_operand:SI 0 "nonimmediate_operand" "=rm")
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand" "=rm")
(ashiftrt:SI (match_operand:SI 1 "general_operand" "0")
(const_int 2)))]
""
@@ -903,7 +908,7 @@
)
(define_expand "ashrsi3"
- [(set (match_operand:SI 0 "nonimmediate_operand")
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand")
(ashiftrt:SI (match_operand:SI 1 "general_operand")
(match_operand:SI 2 "general_operand")))]
""
@@ -912,7 +917,7 @@
)
(define_expand "ashrdi3"
- [(set (match_operand:DI 0 "nonimmediate_operand")
+ [(set (match_operand:DI 0 "msp430_general_dst_nonv_operand")
(ashift:DI (match_operand:DI 1 "general_operand")
(match_operand:DI 2 "general_operand")))]
""
@@ -928,7 +933,7 @@
;; unsigned A >> C
(define_expand "lshrhi3"
- [(set (match_operand:HI 0 "nonimmediate_operand")
+ [(set (match_operand:HI 0 "msp430_general_dst_nonv_operand")
(lshiftrt:HI (match_operand:HI 1 "general_operand")
(match_operand:HI 2 "general_operand")))]
""
@@ -952,7 +957,7 @@
)
(define_insn "srli_1"
- [(set (match_operand:HI 0 "nonimmediate_operand" "=rm")
+ [(set (match_operand:HI 0 "msp430_general_dst_nonv_operand" "=rm")
(lshiftrt:HI (match_operand:HI 1 "general_operand" "0")
(const_int 1)))]
""
@@ -978,7 +983,7 @@
)
(define_insn "srll_1"
- [(set (match_operand:SI 0 "nonimmediate_operand" "=rm")
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand" "=rm")
(lshiftrt:SI (match_operand:SI 1 "general_operand" "0")
(const_int 1)))]
""
@@ -986,7 +991,7 @@
)
(define_insn "srll_2x"
- [(set (match_operand:SI 0 "nonimmediate_operand" "=r")
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand" "=r")
(lshiftrt:SI (match_operand:SI 1 "general_operand" "0")
(const_int 2)))]
"msp430x"
@@ -994,7 +999,7 @@
)
(define_expand "lshrsi3"
- [(set (match_operand:SI 0 "nonimmediate_operand")
+ [(set (match_operand:SI 0 "msp430_general_dst_nonv_operand")
(lshiftrt:SI (match_operand:SI 1 "general_operand")
(match_operand:SI 2 "general_operand")))]
""
@@ -1003,7 +1008,7 @@
)
(define_expand "lshrdi3"
- [(set (match_operand:DI 0 "nonimmediate_operand")
+ [(set (match_operand:DI 0 "msp430_general_dst_nonv_operand")
(ashift:DI (match_operand:DI 1 "general_operand")
(match_operand:DI 2 "general_operand")))]
""
@@ -1274,7 +1279,7 @@
(define_insn "*bitbranch<mode>4"
[(set (pc) (if_then_else
- (ne (and:QHI (match_operand:QHI 0 "msp430_nonimmediate_operand" "rYsYx,rm")
+ (ne (and:QHI (match_operand:QHI 0 "msp430_general_dst_operand" "rYsYx,rm")
(match_operand:QHI 1 "msp430_general_operand" "rYsYxi,rmi"))
(const_int 0))
(label_ref (match_operand 2 "" ""))
@@ -1289,7 +1294,7 @@
(define_insn "*bitbranch<mode>4"
[(set (pc) (if_then_else
- (eq (and:QHI (match_operand:QHI 0 "msp430_nonimmediate_operand" "rYsYx,rm")
+ (eq (and:QHI (match_operand:QHI 0 "msp430_general_dst_operand" "rYsYx,rm")
(match_operand:QHI 1 "msp430_general_operand" "rYsYxi,rmi"))
(const_int 0))
(label_ref (match_operand 2 "" ""))
@@ -1304,7 +1309,7 @@
(define_insn "*bitbranch<mode>4"
[(set (pc) (if_then_else
- (eq (and:QHI (match_operand:QHI 0 "msp430_nonimmediate_operand" "rYsYx,rm")
+ (eq (and:QHI (match_operand:QHI 0 "msp430_general_dst_operand" "rYsYx,rm")
(match_operand:QHI 1 "msp430_general_operand" "rYsYxi,rmi"))
(const_int 0))
(pc)
@@ -1319,7 +1324,7 @@
(define_insn "*bitbranch<mode>4"
[(set (pc) (if_then_else
- (ne (and:QHI (match_operand:QHI 0 "msp430_nonimmediate_operand" "rYsYx,rm")
+ (ne (and:QHI (match_operand:QHI 0 "msp430_general_dst_operand" "rYsYx,rm")
(match_operand:QHI 1 "msp430_general_operand" "rYsYxi,rmi"))
(const_int 0))
(pc)
@@ -1337,7 +1342,7 @@
(define_insn "*bitbranch<mode>4_z"
[(set (pc) (if_then_else
- (ne (zero_extract:HI (match_operand:QHI 0 "msp430_nonimmediate_operand" "rYs,rm")
+ (ne (zero_extract:HI (match_operand:QHI 0 "msp430_general_dst_operand" "rYs,rm")
(const_int 1)
(match_operand 1 "msp430_bitpos" "i,i"))
(const_int 0))
@@ -1353,7 +1358,7 @@
(define_insn "*bitbranch<mode>4_z"
[(set (pc) (if_then_else
- (eq (zero_extract:HI (match_operand:QHI 0 "msp430_nonimmediate_operand" "rm")
+ (eq (zero_extract:HI (match_operand:QHI 0 "msp430_general_dst_operand" "rm")
(const_int 1)
(match_operand 1 "msp430_bitpos" "i"))
(const_int 0))
@@ -1367,7 +1372,7 @@
(define_insn "*bitbranch<mode>4_z"
[(set (pc) (if_then_else
- (eq (zero_extract:HI (match_operand:QHI 0 "msp430_nonimmediate_operand" "rm")
+ (eq (zero_extract:HI (match_operand:QHI 0 "msp430_general_dst_operand" "rm")
(const_int 1)
(match_operand 1 "msp430_bitpos" "i"))
(const_int 0))
@@ -1381,7 +1386,7 @@
(define_insn "*bitbranch<mode>4_z"
[(set (pc) (if_then_else
- (ne (zero_extract:HI (match_operand:QHI 0 "msp430_nonimmediate_operand" "rm")
+ (ne (zero_extract:HI (match_operand:QHI 0 "msp430_general_dst_operand" "rm")
(const_int 1)
(match_operand 1 "msp430_bitpos" "i"))
(const_int 0))
diff --git a/gcc/config/msp430/predicates.md b/gcc/config/msp430/predicates.md
index 751548c4ae8..d8cdaba3813 100644
--- a/gcc/config/msp430/predicates.md
+++ b/gcc/config/msp430/predicates.md
@@ -23,21 +23,50 @@
(match_test ("memory_address_addr_space_p (GET_MODE (op), XEXP (op, 0), MEM_ADDR_SPACE (op))")))
)
+; TRUE if neither op nor op0 are a post_inc. We cannot use post_inc for the
+; dst operand so this must be used for any predicates which might allow a mem.
+; Since we check both op and op0, this will be FALSE for both "(post_inc)" and
+; "(mem (post_inc))"
+(define_predicate "msp430_nonpostinc_operand"
+ (not (ior (match_code "post_inc")
+ (and (ior (match_operand 0 "msp430_volatile_memory_operand")
+ (match_code "mem"))
+ (match_code "post_inc" "0")))))
+
; TRUE for any valid general operand. We do this because
; general_operand refuses to match volatile memory refs.
-
(define_predicate "msp430_general_operand"
(ior (match_operand 0 "general_operand")
(match_operand 0 "msp430_volatile_memory_operand"))
)
; Likewise for nonimmediate_operand.
-
(define_predicate "msp430_nonimmediate_operand"
(ior (match_operand 0 "nonimmediate_operand")
(match_operand 0 "msp430_volatile_memory_operand"))
)
+; Similar to msp430_nonimmediate_operand but disallow post_inc operands
+(define_predicate "msp430_general_dst_operand"
+ (and (match_operand 0 "msp430_nonpostinc_operand")
+ (match_operand 0 "msp430_nonimmediate_operand")))
+
+; Similar to msp430_general_dst_operand but disallow volatile memory references
+; Note that msp430_nonpostinc_operand will allow a volatile mem but nonimmediate
+; will not, so overall this predicate will behave as expected.
+; The heuristic for deciding if we can allow volatile memory appears to be:
+; "If the number of references to the variable in the source code matches
+; the number of references to the variable in the assembly template, we can
+; safely allow a volatile memory reference".
+; - paraphrasing DJ Delorie here:
+; https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00870.html
+; When applied to instruction patterns, this means that we can only allow
+; volatile memory when the output assembler template contains only one
+; instruction which references that volatile address.
+(define_predicate "msp430_general_dst_nonv_operand"
+ (and (match_operand 0 "msp430_nonpostinc_operand")
+ (match_operand 0 "nonimmediate_operand")))
+
(define_predicate "ubyte_operand"
(and (match_code "const_int")
(match_test "IN_RANGE (INTVAL (op), 0, 255)")))
@@ -70,13 +99,20 @@
|| INTVAL (op) == ~8
|| INTVAL (op) == ~(-1) "))))
-(define_predicate "msp430_nonsubreg_operand"
- (match_code "reg,mem"))
+; See above note on post_inc
+(define_predicate "msp430_nonsubreg_dst_operand"
+ (and (match_operand 0 "msp430_nonpostinc_operand")
+ (match_code "reg,mem")))
(define_predicate "msp430_nonsubreg_or_imm_operand"
- (ior (match_operand 0 "msp430_nonsubreg_operand")
+ (ior (match_code "reg,mem")
(match_operand 0 "immediate_operand")))
+(define_predicate "msp430_nonsubregnonpostinc_or_imm_operand"
+ (and (match_operand 0 "msp430_nonpostinc_operand")
+ (ior (match_code "reg,mem")
+ (match_operand 0 "immediate_operand"))))
+
; TRUE for constants which are bit positions for zero_extract
(define_predicate "msp430_bitpos"
(and (match_code "const_int")
--
2.17.1
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][MSP430] Add support for post increment addressing
2019-10-07 20:28 [PATCH][MSP430] Add support for post increment addressing Jozef Lawrynowicz
@ 2019-10-13 16:29 ` Jeff Law
2019-10-14 13:55 ` Jozef Lawrynowicz
0 siblings, 1 reply; 4+ messages in thread
From: Jeff Law @ 2019-10-13 16:29 UTC (permalink / raw)
To: Jozef Lawrynowicz, gcc-patches
On 10/7/19 2:28 PM, Jozef Lawrynowicz wrote:
> MSP430 supports post increment addressing for the source operand only. The
> attached patch enables post increment addressing for MSP430 in GCC.
>
> Regtested on trunk for the small and large memory models.
>
> Ok for trunk?
>
>
> 0001-MSP430-Implement-post-increment-addressing.patch
>
> From d75e48ba434d7bab141c09d442793b2cb26afa69 Mon Sep 17 00:00:00 2001
> From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
> Date: Mon, 16 Sep 2019 16:34:51 +0100
> Subject: [PATCH] MSP430: Implement post increment addressing
>
> gcc/ChangeLog:
>
> 2019-10-07 Jozef Lawrynowicz <jozef.l@mittosystems.com>
>
> * config/msp430/constraints.md: Allow post_inc operand for "Ya"
> constraint.
> * config/msp430/msp430.c (msp430_legitimate_address_p): Handle
> POST_INC.
> (msp430_subreg): Likewise.
> (msp430_split_addsi): Likewise.
> (msp430_print_operand_addr): Likewise.
> * config/msp430/msp430.h (HAVE_POST_INCREMENT): Define.
> (USE_STORE_POST_INCREMENT): Define.
> * config/msp430/msp430.md: Use the msp430_general_dst_operand or
> msp430_general_dst_nonv_operand predicates for the lvalues insns.
> * config/msp430/predicates.md (msp430_nonpostinc_operand): New.
> (msp430_general_dst_operand): New.
> (msp430_general_dst_nonv_operand): New.
> (msp430_nonsubreg_operand): Remove.
> (msp430_nonsubreg_dst_operand): New.
> (msp430_nonsubreg_or_imm_operand): Allow reg or mem operands in place
> of defunct msp430_nonsubreg_operand.
> (msp430_nonsubregnonpostinc_or_imm_operand): New.
So a high level question. The
USE_STORE_{PRE,POST}_{INCREMENT,DECREMENT} are primarily used within the
ivopts code to tune the addressing modes generated in there.
To the best of my knowledge, they do not totally prevent using those
addressing modes elsewhere (auto-inc-dec.c) which instead rely strictly
on the HAVE_ macros and recognizing the result against the MD file.
So will these changes handle auto-increment addressing modes in
destination operands if they are generated by auto-inc-dec.c? Or have
you fixed all the predicates so that auto-inc-dec.c won't create them in
the first place?
Based on a comment in msp430_split_addsi you have to handle stuff like
> + (set (reg:SI)
> + (plus:SI (reg:SI)
> + (mem:SI (post_inc:PSI (reg:PSI))))).
Do you need to check for and do anything special if the destination
operand is the same as the post-inc operand. Note RTX equality test is
probably not sufficient since you've got differing modes. Note this may
be affected by the prior question...
Are there any places where you could potentially have two source memory
operands? If so, do you need additional checks in those patterns?
I've got no conceptual problem with the patch, I just want to make sure
you've thought about those issues.
jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][MSP430] Add support for post increment addressing
2019-10-13 16:29 ` Jeff Law
@ 2019-10-14 13:55 ` Jozef Lawrynowicz
2019-10-14 20:19 ` Jeff Law
0 siblings, 1 reply; 4+ messages in thread
From: Jozef Lawrynowicz @ 2019-10-14 13:55 UTC (permalink / raw)
To: Jeff Law; +Cc: gcc-patches
On Sun, 13 Oct 2019 10:22:31 -0600
Jeff Law <law@redhat.com> wrote:
> On 10/7/19 2:28 PM, Jozef Lawrynowicz wrote:
> > MSP430 supports post increment addressing for the source operand only. The
> > attached patch enables post increment addressing for MSP430 in GCC.
> >
> > Regtested on trunk for the small and large memory models.
> >
> > Ok for trunk?
> >
> >
> > 0001-MSP430-Implement-post-increment-addressing.patch
> >
> > From d75e48ba434d7bab141c09d442793b2cb26afa69 Mon Sep 17 00:00:00 2001
> > From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
> > Date: Mon, 16 Sep 2019 16:34:51 +0100
> > Subject: [PATCH] MSP430: Implement post increment addressing
> >
> > gcc/ChangeLog:
> >
> > 2019-10-07 Jozef Lawrynowicz <jozef.l@mittosystems.com>
> >
> > * config/msp430/constraints.md: Allow post_inc operand for "Ya"
> > constraint.
> > * config/msp430/msp430.c (msp430_legitimate_address_p): Handle
> > POST_INC.
> > (msp430_subreg): Likewise.
> > (msp430_split_addsi): Likewise.
> > (msp430_print_operand_addr): Likewise.
> > * config/msp430/msp430.h (HAVE_POST_INCREMENT): Define.
> > (USE_STORE_POST_INCREMENT): Define.
> > * config/msp430/msp430.md: Use the msp430_general_dst_operand or
> > msp430_general_dst_nonv_operand predicates for the lvalues insns.
> > * config/msp430/predicates.md (msp430_nonpostinc_operand): New.
> > (msp430_general_dst_operand): New.
> > (msp430_general_dst_nonv_operand): New.
> > (msp430_nonsubreg_operand): Remove.
> > (msp430_nonsubreg_dst_operand): New.
> > (msp430_nonsubreg_or_imm_operand): Allow reg or mem operands in place
> > of defunct msp430_nonsubreg_operand.
> > (msp430_nonsubregnonpostinc_or_imm_operand): New.
> So a high level question. The
> USE_STORE_{PRE,POST}_{INCREMENT,DECREMENT} are primarily used within the
> ivopts code to tune the addressing modes generated in there.
>
> To the best of my knowledge, they do not totally prevent using those
> addressing modes elsewhere (auto-inc-dec.c) which instead rely strictly
> on the HAVE_ macros and recognizing the result against the MD file.
Additionally I don't think inc/dec RTXs will ever actually be generated unless
they are handled in TARGET_LEGITIMATE_ADDRESS_P.
>
> So will these changes handle auto-increment addressing modes in
> destination operands if they are generated by auto-inc-dec.c? Or have
> you fixed all the predicates so that auto-inc-dec.c won't create them in
> the first place?
Yes, the new msp430_*_dst_operand predicates I've added will disallow a
(mem (post_inc)).
>
>
>
> Based on a comment in msp430_split_addsi you have to handle stuff like
>
> > + (set (reg:SI)
> > + (plus:SI (reg:SI)
> > + (mem:SI (post_inc:PSI (reg:PSI))))).
>
> Do you need to check for and do anything special if the destination
> operand is the same as the post-inc operand. Note RTX equality test is
> probably not sufficient since you've got differing modes. Note this may
> be affected by the prior question...
This shouldn't ever happen since every lvalue operand which could be a mem has a
dst_operand predicate. msp430_legitimate_address_p only allows post_inc in the
format (post_inc (reg)) so we'll only ever get (mem (post_inc (reg))) RTXs.
>
> Are there any places where you could potentially have two source memory
> operands? If so, do you need additional checks in those patterns?
Are you referring to places where we have two source memory operands, neither
of which are equal to the dest?
We don't have any insns that match this, since all msp430 instructions are
either single or double operand.
For expand patterns that could have two source memory operands,
msp430_expand_helper will always move the operands into registers before
calling a helper function.
And the splitter for addsi is the only other place where we could have two
source memory operands. This is where I put in two predicates which disallow
post_inc, so there is only one possible position we can have it. IIRC, this case
was actually exposed by running the testsuite :)
Also, the assembler will emit the following error message if we ever do
generate a post_inc for the dest operand:
> t.s: Assembler messages:
> t.s:2: Error: this addressing mode is not applicable for destination operand
So I'm quite satisfied that can't/doesn't happen.
Thanks,
Jozef
>
>
> I've got no conceptual problem with the patch, I just want to make sure
> you've thought about those issues.
>
> jeff
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH][MSP430] Add support for post increment addressing
2019-10-14 13:55 ` Jozef Lawrynowicz
@ 2019-10-14 20:19 ` Jeff Law
0 siblings, 0 replies; 4+ messages in thread
From: Jeff Law @ 2019-10-14 20:19 UTC (permalink / raw)
To: Jozef Lawrynowicz; +Cc: gcc-patches
On 10/14/19 7:30 AM, Jozef Lawrynowicz wrote:
> On Sun, 13 Oct 2019 10:22:31 -0600
> Jeff Law <law@redhat.com> wrote:
>
>> On 10/7/19 2:28 PM, Jozef Lawrynowicz wrote:
>>> MSP430 supports post increment addressing for the source operand only. The
>>> attached patch enables post increment addressing for MSP430 in GCC.
>>>
>>> Regtested on trunk for the small and large memory models.
>>>
>>> Ok for trunk?
>>>
>>>
>>> 0001-MSP430-Implement-post-increment-addressing.patch
>>>
>>> From d75e48ba434d7bab141c09d442793b2cb26afa69 Mon Sep 17 00:00:00 2001
>>> From: Jozef Lawrynowicz <jozef.l@mittosystems.com>
>>> Date: Mon, 16 Sep 2019 16:34:51 +0100
>>> Subject: [PATCH] MSP430: Implement post increment addressing
>>>
>>> gcc/ChangeLog:
>>>
>>> 2019-10-07 Jozef Lawrynowicz <jozef.l@mittosystems.com>
>>>
>>> * config/msp430/constraints.md: Allow post_inc operand for "Ya"
>>> constraint.
>>> * config/msp430/msp430.c (msp430_legitimate_address_p): Handle
>>> POST_INC.
>>> (msp430_subreg): Likewise.
>>> (msp430_split_addsi): Likewise.
>>> (msp430_print_operand_addr): Likewise.
>>> * config/msp430/msp430.h (HAVE_POST_INCREMENT): Define.
>>> (USE_STORE_POST_INCREMENT): Define.
>>> * config/msp430/msp430.md: Use the msp430_general_dst_operand or
>>> msp430_general_dst_nonv_operand predicates for the lvalues insns.
>>> * config/msp430/predicates.md (msp430_nonpostinc_operand): New.
>>> (msp430_general_dst_operand): New.
>>> (msp430_general_dst_nonv_operand): New.
>>> (msp430_nonsubreg_operand): Remove.
>>> (msp430_nonsubreg_dst_operand): New.
>>> (msp430_nonsubreg_or_imm_operand): Allow reg or mem operands in place
>>> of defunct msp430_nonsubreg_operand.
>>> (msp430_nonsubregnonpostinc_or_imm_operand): New.
>> So a high level question. The
>> USE_STORE_{PRE,POST}_{INCREMENT,DECREMENT} are primarily used within the
>> ivopts code to tune the addressing modes generated in there.
>>
>> To the best of my knowledge, they do not totally prevent using those
>> addressing modes elsewhere (auto-inc-dec.c) which instead rely strictly
>> on the HAVE_ macros and recognizing the result against the MD file.
>
> Additionally I don't think inc/dec RTXs will ever actually be generated unless
> they are handled in TARGET_LEGITIMATE_ADDRESS_P.
>
>>
>> So will these changes handle auto-increment addressing modes in
>> destination operands if they are generated by auto-inc-dec.c? Or have
>> you fixed all the predicates so that auto-inc-dec.c won't create them in
>> the first place?
>
> Yes, the new msp430_*_dst_operand predicates I've added will disallow a
> (mem (post_inc)).
>
>>
>>
>>
>> Based on a comment in msp430_split_addsi you have to handle stuff like
>>
>>> + (set (reg:SI)
>>> + (plus:SI (reg:SI)
>>> + (mem:SI (post_inc:PSI (reg:PSI))))).
>>
>> Do you need to check for and do anything special if the destination
>> operand is the same as the post-inc operand. Note RTX equality test is
>> probably not sufficient since you've got differing modes. Note this may
>> be affected by the prior question...
>
> This shouldn't ever happen since every lvalue operand which could be a mem has a
> dst_operand predicate. msp430_legitimate_address_p only allows post_inc in the
> format (post_inc (reg)) so we'll only ever get (mem (post_inc (reg))) RTXs.
>>
>> Are there any places where you could potentially have two source memory
>> operands? If so, do you need additional checks in those patterns?
>
> Are you referring to places where we have two source memory operands, neither
> of which are equal to the dest?
Yes.
>
> We don't have any insns that match this, since all msp430 instructions are
> either single or double operand.
Perfect.
>
> For expand patterns that could have two source memory operands,
> msp430_expand_helper will always move the operands into registers before
> calling a helper function.
>
> And the splitter for addsi is the only other place where we could have two
> source memory operands. This is where I put in two predicates which disallow
> post_inc, so there is only one possible position we can have it. IIRC, this case
> was actually exposed by running the testsuite :)
>
> Also, the assembler will emit the following error message if we ever do
> generate a post_inc for the dest operand:
>> t.s: Assembler messages:
>> t.s:2: Error: this addressing mode is not applicable for destination operand
>
> So I'm quite satisfied that can't/doesn't happen.
Perfect. THanks for walking me through things. OK for the trunk.
jeff
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-10-14 20:13 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-07 20:28 [PATCH][MSP430] Add support for post increment addressing Jozef Lawrynowicz
2019-10-13 16:29 ` Jeff Law
2019-10-14 13:55 ` Jozef Lawrynowicz
2019-10-14 20:19 ` Jeff Law
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).