* [committed] Fix minor H8 bug and prepare for more redundant test/compare elimination
@ 2021-06-02 5:04 Jeff Law
0 siblings, 0 replies; only message in thread
From: Jeff Law @ 2021-06-02 5:04 UTC (permalink / raw)
To: GCC Patches
[-- Attachment #1: Type: text/plain, Size: 1150 bytes --]
These are some minor changes to the H8 port to fix a latent bug and
prepare for having logical operations participate in redundant
test/compare elimination that I've been sitting on for a while.
These have been through my tester with testing timeouts dramatically
increased to give as much coverage as possible (runs are taking nearly
18 hours, even after some hacks to dramatically improve the long-running
__builtin_<op>_overflow tests).
Conceptually the idea here is to move away from using match_operator and
instead towards an iterator for the logical ops. That allows me to use
the existing define_subst to enable redundant test/compare elimination
in a later patch. Along the way this also starts to consolidate the
QImode logicals with the HI/SI mode logicals and consolidate AND
handling with IOR/XOR.
There's one bugfix buried in here. In particular the
define_insn_and_split patterns were using "reload_completed" in their
split condition. It should have been "&& reload_completed". I strongly
suspect there's other cases of this bug lurking and fixing them is on my
TODO list.
Installing on the trunk,
Jeff
[-- Attachment #2: P --]
[-- Type: text/plain, Size: 14185 bytes --]
commit 40e484885c10a0c8bd994c5cf7bf247998a4ad6b
Author: Jeff Law <jeffreyalaw@gmail.com>
Date: Wed Jun 2 00:56:38 2021 -0400
Fix minor bugs in H8 port logical ops. Prepare for more compare/test removal
gcc/
* config/h8300/h8300-protos. (compute_a_shift_length): Drop unused
argument from prototype.
(output_logical_op): Add rtx_code argument.
(compute_logical_op_length): Likewise.
* config/h8300/h8300.c (h8300_and_costs): Pass additional argument
to compute_a_shift_length.
(output_logical_op); New argument with the rtx code rather than
extracting it from an operand. Handle QImode too.
(compute_logical_op_length): Similary.
(compute_a_shift_length): Drop unused argument.
* config/h8300/h8300.md (logicals): New code iterator.
* config/h8300/logical.md (<code><mode>3 expander): Combine
the "and" expander with the "ior"/"xor" expander.
(bclr<mode>msx): Combine the QI/HI mode patterns.
(<logical><mode>3 insns): Use code iterator rather than match_operator.
Handle QImode as well. Update call to output_logical_op and
compute_logical_op_length to pass in rtx_code
Fix split condition on all define_insn_and_split patterns.
(one_cmpl<mode>2<cczn>): Use <cczn> to support both clobbering
the flags and setting ZN via existing define_subst.
* config/h8300/shiftrotate.md: Drop unused argument from
calls to compute_a_shift_length.
Signed-off-by: Jeff Law <jeffreyalaw@gmail.com>
diff --git a/gcc/config/h8300/h8300-protos.h b/gcc/config/h8300/h8300-protos.h
index 45e7dec3c7d..af653292a9d 100644
--- a/gcc/config/h8300/h8300-protos.h
+++ b/gcc/config/h8300/h8300-protos.h
@@ -29,16 +29,15 @@ extern unsigned int compute_mov_length (rtx *);
extern const char *output_plussi (rtx *, bool);
extern unsigned int compute_plussi_length (rtx *, bool);
extern const char *output_a_shift (rtx *);
-extern unsigned int compute_a_shift_length (rtx, rtx *);
+extern unsigned int compute_a_shift_length (rtx *);
extern const char *output_a_rotate (enum rtx_code, rtx *);
extern unsigned int compute_a_rotate_length (rtx *);
extern const char *output_simode_bld (int, rtx[]);
extern void final_prescan_insn (rtx_insn *, rtx *, int);
extern int h8300_expand_movsi (rtx[]);
extern machine_mode h8300_select_cc_mode (RTX_CODE, rtx, rtx);
-extern const char *output_logical_op (machine_mode, rtx *);
-extern unsigned int compute_logical_op_length (machine_mode,
- rtx *);
+extern const char *output_logical_op (machine_mode, rtx_code code, rtx *);
+extern unsigned int compute_logical_op_length (machine_mode, rtx_code, rtx *);
extern int compute_logical_op_cc (machine_mode, rtx *);
extern int compute_a_shift_cc (rtx, rtx *);
diff --git a/gcc/config/h8300/h8300.c b/gcc/config/h8300/h8300.c
index ba2b9daf487..ef947aa468a 100644
--- a/gcc/config/h8300/h8300.c
+++ b/gcc/config/h8300/h8300.c
@@ -1100,7 +1100,7 @@ h8300_and_costs (rtx x)
operands[1] = XEXP (x, 0);
operands[2] = XEXP (x, 1);
operands[3] = x;
- return compute_logical_op_length (GET_MODE (x), operands) / 2;
+ return compute_logical_op_length (GET_MODE (x), AND, operands) / 2;
}
/* Compute the cost of a shift insn. */
@@ -1119,7 +1119,7 @@ h8300_shift_costs (rtx x)
operands[1] = NULL;
operands[2] = XEXP (x, 1);
operands[3] = x;
- return compute_a_shift_length (NULL, operands) / 2;
+ return compute_a_shift_length (operands) / 2;
}
/* Worker function for TARGET_RTX_COSTS. */
@@ -2879,10 +2879,8 @@ compute_plussi_cc (rtx *operands)
/* Output a logical insn. */
const char *
-output_logical_op (machine_mode mode, rtx *operands)
+output_logical_op (machine_mode mode, rtx_code code, rtx *operands)
{
- /* Figure out the logical op that we need to perform. */
- enum rtx_code code = GET_CODE (operands[3]);
/* Pretend that every byte is affected if both operands are registers. */
const unsigned HOST_WIDE_INT intval =
(unsigned HOST_WIDE_INT) ((GET_CODE (operands[2]) == CONST_INT)
@@ -2923,6 +2921,10 @@ output_logical_op (machine_mode mode, rtx *operands)
switch (mode)
{
+ case E_QImode:
+ sprintf (insn_buf, "%s.b\t%%X2,%%X0", opname);
+ output_asm_insn (insn_buf, operands);
+ break;
case E_HImode:
/* First, see if we can finish with one insn. */
if (b0 != 0 && b1 != 0)
@@ -3033,10 +3035,8 @@ output_logical_op (machine_mode mode, rtx *operands)
/* Compute the length of a logical insn. */
unsigned int
-compute_logical_op_length (machine_mode mode, rtx *operands)
+compute_logical_op_length (machine_mode mode, rtx_code code, rtx *operands)
{
- /* Figure out the logical op that we need to perform. */
- enum rtx_code code = GET_CODE (operands[3]);
/* Pretend that every byte is affected if both operands are registers. */
const unsigned HOST_WIDE_INT intval =
(unsigned HOST_WIDE_INT) ((GET_CODE (operands[2]) == CONST_INT)
@@ -3061,6 +3061,9 @@ compute_logical_op_length (machine_mode mode, rtx *operands)
switch (mode)
{
+ case E_QImode:
+ return 2;
+
case E_HImode:
/* First, see if we can finish with one insn. */
if (b0 != 0 && b1 != 0)
@@ -4189,7 +4192,7 @@ h8300_asm_insn_count (const char *templ)
/* Compute the length of a shift insn. */
unsigned int
-compute_a_shift_length (rtx insn ATTRIBUTE_UNUSED, rtx *operands)
+compute_a_shift_length (rtx *operands)
{
rtx shift = operands[3];
machine_mode mode = GET_MODE (shift);
diff --git a/gcc/config/h8300/h8300.md b/gcc/config/h8300/h8300.md
index 9a42547a92c..e596987a6a6 100644
--- a/gcc/config/h8300/h8300.md
+++ b/gcc/config/h8300/h8300.md
@@ -229,6 +229,8 @@
(define_code_iterator shifts [ashift ashiftrt lshiftrt])
+(define_code_iterator logicals [ior xor and])
+
(define_code_iterator ors [ior xor])
\f
(include "movepush.md")
diff --git a/gcc/config/h8300/logical.md b/gcc/config/h8300/logical.md
index eb99c20b55d..d778d24c580 100644
--- a/gcc/config/h8300/logical.md
+++ b/gcc/config/h8300/logical.md
@@ -1,11 +1,20 @@
+;; Generic for binary logicals across the supported integer modes
+(define_expand "<code><mode>3"
+ [(set (match_operand:QHSI 0 "register_operand" "")
+ (logicals:QHSI (match_operand:QHSI 1 "register_operand" "")
+ (match_operand:QHSI 2 "h8300_src_operand" "")))]
+ ""
+ "")
+
+;; There's a ton of cleanup to do from here below.
;; ----------------------------------------------------------------------
;; AND INSTRUCTIONS
;; ----------------------------------------------------------------------
-(define_insn "bclrqi_msx"
- [(set (match_operand:QI 0 "bit_register_indirect_operand" "=WU")
- (and:QI (match_operand:QI 1 "bit_register_indirect_operand" "%0")
- (match_operand:QI 2 "single_zero_operand" "Y0")))]
+(define_insn "bclr<mode>_msx"
+ [(set (match_operand:QHI 0 "bit_register_indirect_operand" "=WU")
+ (and:QHI (match_operand:QHI 1 "bit_register_indirect_operand" "%0")
+ (match_operand:QHI 2 "single_zero_operand" "Y0")))]
"TARGET_H8300SX && rtx_equal_p (operands[0], operands[1])"
"bclr\\t%W2,%0"
[(set_attr "length" "8")])
@@ -24,21 +33,13 @@
operands[2] = GEN_INT ((INTVAL (operands[2])) >> 8);
})
-(define_insn "bclrhi_msx"
- [(set (match_operand:HI 0 "bit_register_indirect_operand" "=m")
- (and:HI (match_operand:HI 1 "bit_register_indirect_operand" "%0")
- (match_operand:HI 2 "single_zero_operand" "Y0")))]
- "TARGET_H8300SX"
- "bclr\\t%W2,%0"
- [(set_attr "length" "8")])
-
(define_insn_and_split "*andqi3_2"
[(set (match_operand:QI 0 "bit_operand" "=U,rQ,r")
(and:QI (match_operand:QI 1 "bit_operand" "%0,0,WU")
(match_operand:QI 2 "h8300_src_operand" "Y0,rQi,IP1>X")))]
"TARGET_H8300SX"
"#"
- "reload_completed"
+ "&& reload_completed"
[(parallel [(set (match_dup 0) (and:QI (match_dup 1) (match_dup 2)))
(clobber (reg:CC CC_REG))])])
@@ -62,7 +63,7 @@
"register_operand (operands[0], QImode)
|| single_zero_operand (operands[2], QImode)"
"#"
- "reload_completed"
+ "&& reload_completed"
[(parallel [(set (match_dup 0) (and:QI (match_dup 1) (match_dup 2)))
(clobber (reg:CC CC_REG))])])
@@ -78,13 +79,6 @@
and %X2,%X0"
[(set_attr "length" "2,8")])
-(define_expand "and<mode>3"
- [(set (match_operand:QHSI 0 "register_operand" "")
- (and:QHSI (match_operand:QHSI 1 "register_operand" "")
- (match_operand:QHSI 2 "h8300_src_operand" "")))]
- ""
- "")
-
(define_insn_and_split "*andor<mode>3"
[(set (match_operand:QHSI 0 "register_operand" "=r")
(ior:QHSI (and:QHSI (match_operand:QHSI 2 "register_operand" "r")
@@ -95,7 +89,7 @@
|| (<MODE>mode == SImode
&& (INTVAL (operands[3]) & 0xffff) != 0))"
"#"
- "reload_completed"
+ "&& reload_completed"
[(parallel [(set (match_dup 0) (ior:QHSI (and:QHSI (match_dup 2)
(match_dup 3))
(match_dup 1)))
@@ -150,7 +144,7 @@
(match_operand:SI 1 "register_operand" "0")))]
""
"#"
- "reload_completed"
+ "&& reload_completed"
[(parallel [(set (match_dup 0) (ior:SI (and:SI (ashift:SI (match_dup 2)
(const_int 8))
(const_int 65280))
@@ -195,7 +189,7 @@
"TARGET_H8300SX || register_operand (operands[0], QImode)
|| single_one_operand (operands[2], QImode)"
"#"
- "reload_completed"
+ "&& reload_completed"
[(parallel [(set (match_dup 0) (ors:QI (match_dup 1) (match_dup 2)))
(clobber (reg:CC CC_REG))])])
@@ -216,39 +210,32 @@
[(set_attr "length" "8,*")
(set_attr "length_table" "*,logicb")])
-(define_expand "<code><mode>3"
- [(set (match_operand:QHSI 0 "register_operand" "")
- (ors:QHSI (match_operand:QHSI 1 "register_operand" "")
- (match_operand:QHSI 2 "h8300_src_operand" "")))]
- ""
- "")
-
;; ----------------------------------------------------------------------
;; {AND,IOR,XOR}{HI3,SI3} PATTERNS
;; ----------------------------------------------------------------------
(define_insn_and_split "*logical<mode>3"
- [(set (match_operand:HSI 0 "h8300_dst_operand" "=rQ")
- (match_operator:HSI 3 "bit_operator"
- [(match_operand:HSI 1 "h8300_dst_operand" "%0")
- (match_operand:HSI 2 "h8300_src_operand" "rQi")]))]
+ [(set (match_operand:QHSI 0 "h8300_dst_operand" "=rQ")
+ (logicals:QHSI
+ (match_operand:QHSI 1 "h8300_dst_operand" "%0")
+ (match_operand:QHSI 2 "h8300_src_operand" "rQi")))]
"h8300_operands_match_p (operands)"
"#"
- "reload_completed"
+ "&& reload_completed"
[(parallel [(set (match_dup 0)
(match_op_dup 3 [(match_dup 1) (match_dup 2)]))
(clobber (reg:CC CC_REG))])])
-(define_insn "*logical<mode>3_clobber_flags"
- [(set (match_operand:HSI 0 "h8300_dst_operand" "=rQ")
- (match_operator:HSI 3 "bit_operator"
- [(match_operand:HSI 1 "h8300_dst_operand" "%0")
- (match_operand:HSI 2 "h8300_src_operand" "rQi")]))
+(define_insn "*<code><mode>3_clobber_flags"
+ [(set (match_operand:QHSI 0 "h8300_dst_operand" "=rQ")
+ (logicals:QHSI
+ (match_operand:QHSI 1 "h8300_dst_operand" "%0")
+ (match_operand:QHSI 2 "h8300_src_operand" "rQi")))
(clobber (reg:CC CC_REG))]
"h8300_operands_match_p (operands)"
- { return output_logical_op (<MODE>mode, operands); }
+ { return output_logical_op (<MODE>mode, <CODE>, operands); }
[(set (attr "length")
- (symbol_ref "compute_logical_op_length (<MODE>mode, operands)"))])
+ (symbol_ref "compute_logical_op_length (<MODE>mode, <CODE>, operands)"))])
\f
;; ----------------------------------------------------------------------
@@ -260,11 +247,11 @@
(not:QHSI (match_operand:QHSI 1 "h8300_dst_operand" "0")))]
""
"#"
- "reload_completed"
+ "&& reload_completed"
[(parallel [(set (match_dup 0) (not:QHSI (match_dup 1)))
(clobber (reg:CC CC_REG))])])
-(define_insn "one_cmpl<mode>2_clobber_flags"
+(define_insn "one_cmpl<mode>2_<cczn>"
[(set (match_operand:QHSI 0 "h8300_dst_operand" "=rQ")
(not:QHSI (match_operand:QHSI 1 "h8300_dst_operand" "0")))
(clobber (reg:CC CC_REG))]
diff --git a/gcc/config/h8300/shiftrotate.md b/gcc/config/h8300/shiftrotate.md
index f1c86f7da1c..4bf8fe14e0b 100644
--- a/gcc/config/h8300/shiftrotate.md
+++ b/gcc/config/h8300/shiftrotate.md
@@ -175,7 +175,7 @@
return output_a_shift (operands);
}
[(set (attr "length")
- (symbol_ref "compute_a_shift_length (insn, operands)"))])
+ (symbol_ref "compute_a_shift_length (operands)"))])
(define_insn_and_split "*shiftqi_noscratch"
[(set (match_operand:QI 0 "register_operand" "=r,r")
@@ -203,7 +203,7 @@
return output_a_shift (operands);
}
[(set (attr "length")
- (symbol_ref "compute_a_shift_length (insn, operands)"))])
+ (symbol_ref "compute_a_shift_length (operands)"))])
(define_insn_and_split "*shifthi"
[(set (match_operand:HI 0 "register_operand" "=r,r")
@@ -230,7 +230,7 @@
return output_a_shift (operands);
}
[(set (attr "length")
- (symbol_ref "compute_a_shift_length (insn, operands)"))])
+ (symbol_ref "compute_a_shift_length (operands)"))])
(define_insn_and_split "*shifthi_noscratch"
[(set (match_operand:HI 0 "register_operand" "=r,r")
@@ -258,7 +258,7 @@
return output_a_shift (operands);
}
[(set (attr "length")
- (symbol_ref "compute_a_shift_length (insn, operands)"))])
+ (symbol_ref "compute_a_shift_length (operands)"))])
(define_insn_and_split "*shiftsi"
[(set (match_operand:SI 0 "register_operand" "=r,r")
@@ -285,7 +285,7 @@
return output_a_shift (operands);
}
[(set (attr "length")
- (symbol_ref "compute_a_shift_length (insn, operands)"))])
+ (symbol_ref "compute_a_shift_length (operands)"))])
(define_insn_and_split "*shiftsi_noscratch"
[(set (match_operand:SI 0 "register_operand" "=r,r")
@@ -313,7 +313,7 @@
return output_a_shift (operands);
}
[(set (attr "length")
- (symbol_ref "compute_a_shift_length (insn, operands)"))])
+ (symbol_ref "compute_a_shift_length (operands)"))])
;; Split a variable shift into a loop. If the register containing
;; the shift count dies, then we just use that register.
^ permalink raw reply [flat|nested] only message in thread
only message in thread, other threads:[~2021-06-02 5:04 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-02 5:04 [committed] Fix minor H8 bug and prepare for more redundant test/compare elimination 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).