This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH, ARM] correctly encode the CC reg data flow
- From: Bernd Edlinger <bernd dot edlinger at hotmail dot de>
- To: "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>
- Cc: Ramana Radhakrishnan <ramana dot radhakrishnan at arm dot com>, Richard Earnshaw <richard dot earnshaw at arm dot com>, Kyrill Tkachov <kyrylo dot tkachov at foss dot arm dot com>, Wilco Dijkstra <wilco dot dijkstra at arm dot com>
- Date: Sun, 18 Dec 2016 12:58:53 +0000
- Subject: [PATCH, ARM] correctly encode the CC reg data flow
- Authentication-results: sourceware.org; auth=none
- Authentication-results: gcc.gnu.org; dkim=none (message not signed) header.d=none;gcc.gnu.org; dmarc=none action=none header.from=hotmail.de;
- Spamdiagnosticmetadata: NSPM
- Spamdiagnosticoutput: 1:99
Hi,
this is related to PR77308, the follow-up patch will depend on this one.
When trying the split the *arm_cmpdi_insn and *arm_cmpdi_unsigned
before reload, a mis-compilation in libgcc function __gnu_satfractdasq
was discovered, see [1] for more details.
The reason seems to be that when the *arm_cmpdi_insn is directly
followed by a *arm_cmpdi_unsigned instruction, both are split
up into this:
[(set (reg:CC CC_REGNUM)
(compare:CC (match_dup 0) (match_dup 1)))
(parallel [(set (reg:CC CC_REGNUM)
(compare:CC (match_dup 3) (match_dup 4)))
(set (match_dup 2)
(minus:SI (match_dup 5)
(ltu:SI (reg:CC_C CC_REGNUM) (const_int
0))))])]
[(set (reg:CC CC_REGNUM)
(compare:CC (match_dup 2) (match_dup 3)))
(cond_exec (eq:SI (reg:CC CC_REGNUM) (const_int 0))
(set (reg:CC CC_REGNUM)
(compare:CC (match_dup 0) (match_dup 1))))]
The problem is that the reg:CC from the *subsi3_carryin_compare
is not mentioning that the reg:CC is also dependent on the reg:CC
from before. Therefore the *arm_cmpsi_insn appears to be
redundant and thus got removed, because the data values are identical.
I think that applies to a number of similar pattern where data
flow is happening through the CC reg.
So this is a kind of correctness issue, and should be fixed
independently from the optimization issue PR77308.
Therefore I think the patterns need to specify the true
value that will be in the CC reg, in order for cse to
know what the instructions are really doing.
Bootstrapped and reg-tested on arm-linux-gnueabihf.
Is it OK for trunk?
Thanks
Bernd.
[1] https://gcc.gnu.org/ml/gcc-patches/2016-12/msg00680.html
2016-12-10 Bernd Edlinger <bernd.edlinger@hotmail.de>
PR target/77308
* config/arm/arm.md (adddi3_compareV, *addsi3_compareV_upper,
adddi3_compareC, *addsi3_compareC_upper, subdi3_compare1,
subsi3_carryin_compare, subsi3_carryin_compare_const,
negdi2_compare, *negsi2_carryin_compare,
*arm_cmpdi_insn): Fix the CC reg dataflow.
Index: gcc/config/arm/arm.md
===================================================================
--- gcc/config/arm/arm.md (revision 243515)
+++ gcc/config/arm/arm.md (working copy)
@@ -669,17 +669,15 @@
(set (match_dup 0) (plus:SI (match_dup 1) (match_dup 2)))])
(parallel [(set (reg:CC_V CC_REGNUM)
(ne:CC_V
- (plus:DI (plus:DI
- (sign_extend:DI (match_dup 4))
- (sign_extend:DI (match_dup 5)))
- (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))
- (plus:DI (sign_extend:DI
- (plus:SI (match_dup 4) (match_dup 5)))
- (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
- (set (match_dup 3) (plus:SI (plus:SI
- (match_dup 4) (match_dup 5))
- (ltu:SI (reg:CC_C CC_REGNUM)
- (const_int 0))))])]
+ (plus:DI (plus:DI (sign_extend:DI (match_dup 4))
+ (sign_extend:DI (match_dup 5)))
+ (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))
+ (sign_extend:DI
+ (plus:SI (plus:SI (match_dup 4) (match_dup 5))
+ (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))))
+ (set (match_dup 3) (plus:SI (plus:SI (match_dup 4) (match_dup 5))
+ (ltu:SI (reg:CC_C CC_REGNUM)
+ (const_int 0))))])]
"
{
operands[3] = gen_highpart (SImode, operands[0]);
@@ -713,13 +711,13 @@
[(set (reg:CC_V CC_REGNUM)
(ne:CC_V
(plus:DI
- (plus:DI
- (sign_extend:DI (match_operand:SI 1 "register_operand" "r"))
- (sign_extend:DI (match_operand:SI 2 "register_operand" "r")))
- (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))
- (plus:DI (sign_extend:DI
- (plus:SI (match_dup 1) (match_dup 2)))
- (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
+ (plus:DI
+ (sign_extend:DI (match_operand:SI 1 "register_operand" "r"))
+ (sign_extend:DI (match_operand:SI 2 "register_operand" "r")))
+ (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))
+ (sign_extend:DI (plus:SI (plus:SI (match_dup 1) (match_dup 2))
+ (ltu:SI (reg:CC_C CC_REGNUM)
+ (const_int 0))))))
(set (match_operand:SI 0 "register_operand" "=r")
(plus:SI
(plus:SI (match_dup 1) (match_dup 2))
@@ -748,17 +746,15 @@
(set (match_dup 0) (plus:SI (match_dup 1) (match_dup 2)))])
(parallel [(set (reg:CC_C CC_REGNUM)
(ne:CC_C
- (plus:DI (plus:DI
- (zero_extend:DI (match_dup 4))
- (zero_extend:DI (match_dup 5)))
- (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))
- (plus:DI (zero_extend:DI
- (plus:SI (match_dup 4) (match_dup 5)))
- (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
- (set (match_dup 3) (plus:SI
- (plus:SI (match_dup 4) (match_dup 5))
- (ltu:SI (reg:CC_C CC_REGNUM)
- (const_int 0))))])]
+ (plus:DI (plus:DI (zero_extend:DI (match_dup 4))
+ (zero_extend:DI (match_dup 5)))
+ (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))
+ (zero_extend:DI
+ (plus:SI (plus:SI (match_dup 4) (match_dup 5))
+ (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))))
+ (set (match_dup 3) (plus:SI (plus:SI (match_dup 4) (match_dup 5))
+ (ltu:SI (reg:CC_C CC_REGNUM)
+ (const_int 0))))])]
"
{
operands[3] = gen_highpart (SImode, operands[0]);
@@ -777,17 +773,16 @@
[(set (reg:CC_C CC_REGNUM)
(ne:CC_C
(plus:DI
- (plus:DI
- (zero_extend:DI (match_operand:SI 1 "register_operand" "r"))
- (zero_extend:DI (match_operand:SI 2 "register_operand" "r")))
- (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))
- (plus:DI (zero_extend:DI
- (plus:SI (match_dup 1) (match_dup 2)))
- (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
+ (plus:DI
+ (zero_extend:DI (match_operand:SI 1 "register_operand" "r"))
+ (zero_extend:DI (match_operand:SI 2 "register_operand" "r")))
+ (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))
+ (zero_extend:DI
+ (plus:SI (plus:SI (match_dup 1) (match_dup 2))
+ (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))))
(set (match_operand:SI 0 "register_operand" "=r")
- (plus:SI
- (plus:SI (match_dup 1) (match_dup 2))
- (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+ (plus:SI (plus:SI (match_dup 1) (match_dup 2))
+ (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
"TARGET_32BIT"
"adcs%?\\t%0, %1, %2"
[(set_attr "conds" "set")
@@ -1086,8 +1081,8 @@
})
(define_insn_and_split "subdi3_compare1"
- [(set (reg:CC CC_REGNUM)
- (compare:CC
+ [(set (reg:CC_NCV CC_REGNUM)
+ (compare:CC_NCV
(match_operand:DI 1 "register_operand" "r")
(match_operand:DI 2 "register_operand" "r")))
(set (match_operand:DI 0 "register_operand" "=&r")
@@ -1098,10 +1093,14 @@
[(parallel [(set (reg:CC CC_REGNUM)
(compare:CC (match_dup 1) (match_dup 2)))
(set (match_dup 0) (minus:SI (match_dup 1) (match_dup 2)))])
- (parallel [(set (reg:CC CC_REGNUM)
- (compare:CC (match_dup 4) (match_dup 5)))
- (set (match_dup 3) (minus:SI (minus:SI (match_dup 4) (match_dup 5))
- (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))])]
+ (parallel [(set (reg:CC_C CC_REGNUM)
+ (compare:CC_C
+ (zero_extend:DI (match_dup 4))
+ (plus:DI (zero_extend:DI (match_dup 5))
+ (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
+ (set (match_dup 3)
+ (minus:SI (minus:SI (match_dup 4) (match_dup 5))
+ (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))])]
{
operands[3] = gen_highpart (SImode, operands[0]);
operands[0] = gen_lowpart (SImode, operands[0]);
@@ -1156,13 +1155,15 @@
)
(define_insn "*subsi3_carryin_compare"
- [(set (reg:CC CC_REGNUM)
- (compare:CC (match_operand:SI 1 "s_register_operand" "r")
- (match_operand:SI 2 "s_register_operand" "r")))
+ [(set (reg:CC_C CC_REGNUM)
+ (compare:CC_C
+ (zero_extend:DI (match_operand:SI 1 "s_register_operand" "r"))
+ (plus:DI
+ (zero_extend:DI (match_operand:SI 2 "s_register_operand" "r"))
+ (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
(set (match_operand:SI 0 "s_register_operand" "=r")
- (minus:SI (minus:SI (match_dup 1)
- (match_dup 2))
- (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+ (minus:SI (minus:SI (match_dup 1) (match_dup 2))
+ (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
"TARGET_32BIT"
"sbcs\\t%0, %1, %2"
[(set_attr "conds" "set")
@@ -1170,13 +1171,15 @@
)
(define_insn "*subsi3_carryin_compare_const"
- [(set (reg:CC CC_REGNUM)
- (compare:CC (match_operand:SI 1 "reg_or_int_operand" "r")
- (match_operand:SI 2 "arm_not_operand" "K")))
+ [(set (reg:CC_C CC_REGNUM)
+ (compare:CC_C
+ (zero_extend:DI (match_operand:SI 1 "reg_or_int_operand" "r"))
+ (plus:DI
+ (zero_extend:DI (match_operand:SI 2 "arm_not_operand" "K"))
+ (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
(set (match_operand:SI 0 "s_register_operand" "=r")
- (minus:SI (plus:SI (match_dup 1)
- (match_dup 2))
- (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
+ (minus:SI (plus:SI (match_dup 1) (match_dup 2))
+ (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
"TARGET_32BIT"
"sbcs\\t%0, %1, #%B2"
[(set_attr "conds" "set")
@@ -4633,8 +4636,8 @@
(define_insn_and_split "negdi2_compare"
- [(set (reg:CC CC_REGNUM)
- (compare:CC
+ [(set (reg:CC_NCV CC_REGNUM)
+ (compare:CC_NCV
(const_int 0)
(match_operand:DI 1 "register_operand" "0,r")))
(set (match_operand:DI 0 "register_operand" "=r,&r")
@@ -4646,8 +4649,12 @@
(compare:CC (const_int 0) (match_dup 1)))
(set (match_dup 0) (minus:SI (const_int 0)
(match_dup 1)))])
- (parallel [(set (reg:CC CC_REGNUM)
- (compare:CC (const_int 0) (match_dup 3)))
+ (parallel [(set (reg:CC_C CC_REGNUM)
+ (compare:CC_C
+ (const_int 0)
+ (plus:DI
+ (zero_extend:DI (match_dup 3))
+ (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
(set (match_dup 2)
(minus:SI
(minus:SI (const_int 0) (match_dup 3))
@@ -4705,12 +4712,14 @@
)
(define_insn "*negsi2_carryin_compare"
- [(set (reg:CC CC_REGNUM)
- (compare:CC (const_int 0)
- (match_operand:SI 1 "s_register_operand" "r")))
+ [(set (reg:CC_C CC_REGNUM)
+ (compare:CC_C
+ (const_int 0)
+ (plus:DI
+ (zero_extend:DI (match_operand:SI 1 "s_register_operand" "r"))
+ (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
(set (match_operand:SI 0 "s_register_operand" "=r")
- (minus:SI (minus:SI (const_int 0)
- (match_dup 1))
+ (minus:SI (minus:SI (const_int 0) (match_dup 1))
(ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))]
"TARGET_ARM"
"rscs\\t%0, %1, #0"
@@ -7359,12 +7368,15 @@
"#" ; "cmp\\t%Q0, %Q1\;sbcs\\t%2, %R0, %R1"
"&& reload_completed"
[(set (reg:CC CC_REGNUM)
- (compare:CC (match_dup 0) (match_dup 1)))
- (parallel [(set (reg:CC CC_REGNUM)
- (compare:CC (match_dup 3) (match_dup 4)))
- (set (match_dup 2)
- (minus:SI (match_dup 5)
- (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))])]
+ (compare:CC (match_dup 0) (match_dup 1)))
+ (parallel [(set (reg:CC_C CC_REGNUM)
+ (compare:CC_C
+ (zero_extend:DI (match_dup 3))
+ (plus:DI (zero_extend:DI (match_dup 4))
+ (ltu:DI (reg:CC_C CC_REGNUM) (const_int 0)))))
+ (set (match_dup 2)
+ (minus:SI (match_dup 5)
+ (ltu:SI (reg:CC_C CC_REGNUM) (const_int 0))))])]
{
operands[3] = gen_highpart (SImode, operands[0]);
operands[0] = gen_lowpart (SImode, operands[0]);