[v1] aarch64: Fix issues with 'overflow' insns [PR126463]
Checks
| Context |
Check |
Description |
| linaro-tcwg-bot/tcwg_simplebootstrap_build--master-aarch64-bootstrap |
success
|
Build passed
|
| linaro-tcwg-bot/tcwg_simplebootstrap_build--master-arm-bootstrap |
success
|
Build passed
|
Commit Message
From: Richard Earnshaw <rearnsha@arm.com>
This issue looked like a simple 1-character typo, but when trying to
construct a suitable testcase, I found that the patterns were hardly
ever being used. In the end there turned out to be three underlying
issues:
- we were selecting the wrong CCmode for NEG and MINUS overflow
checks (more precisely, we weren't detecting these at all and
then falling back to CCmode).
- The patterns were not in canonical form, preventing combine
from finding a match while optimizing.
- Finally, when we did eventually get the preferred form, we
had the invalid output template, causing the reported ICE.
gcc/ChangeLog:
PR target/126463
* config/aarch64/aarch64.cc (aarch64_select_cc_mode): Correctly
match overflow detection idioms using MINUS and NEG.
* config/aarch64/aarch64.md (subv<GPI:mode>4): Re-order the
compare into canonical form.
(subv<GPI:mode>4_insn): Likewise.
(negv<GPI:mode>4): Likewise.
(negv<GPI:mode>_insn): Likewise. Fix typo in output template.
gcc/testsuite/ChangeLog:
PR target/126463
* gcc.target/aarch64/negv.c: New test.
---
This is a forge pull request published on the gcc-patches mailing list mailing list
as requested by Richard Earnshaw via Sourceware Forge <forge-bot+rearnsha@forge-stage.sourceware.org>.
Forge discussion: https://forge.sourceware.org/gcc/gcc/pulls/227
Get it locally using:
```
git fetch forge-upstream "+refs/versioned_pull/227/*:refs/versioned_pull/227/*"
git switch -c "pr-227-v1" "refs/versioned_pull/227/1/head"
```
Or, download the patch at: https://forge.sourceware.org/gcc/gcc/pulls/227.diff
Created on: 2026-09-02 16:47:57+00:00
Latest update: 2026-09-02 16:47:57+00:00
Changes: 3 changed files, 52 additions, 23 deletions
Head revision: rearnsha/gcc ref aarch64-cmpv commit a44a7d66cb7dfaf81e8b2db03cc5a4049c148ef7
Base revision: gcc/gcc ref trunk commit 0acb69bf9bb0db14b7e230fb5370257c530fb6e8 r17-3869-g0acb69bf9bb0db
Merge base: 0acb69bf9bb0db14b7e230fb5370257c530fb6e8
Requested Reviewers:
Changed files:
- A: gcc/testsuite/gcc.target/aarch64/negv.c
- M: gcc/config/aarch64/aarch64.cc
- M: gcc/config/aarch64/aarch64.md
gcc/config/aarch64/aarch64.cc | 10 ++++---
gcc/config/aarch64/aarch64.md | 38 ++++++++++++-------------
gcc/testsuite/gcc.target/aarch64/negv.c | 27 ++++++++++++++++++
3 files changed, 52 insertions(+), 23 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/negv.c
Comments
Wilco Dijkstra (Wilco) <wilco.dijkstra@arm.com> approved the changes:
LGTM. OK
--
https://forge.sourceware.org/gcc/gcc/pulls/227#issuecomment-7417
Thanks, committed.
--
https://forge.sourceware.org/gcc/gcc/pulls/227#issuecomment-7418
This is a summary of discussions relative to the merge request created by Richard Earnshaw (rearnsha) <rearnsha@arm.com> titled
aarch64: Fix issues with 'overflow' insns [PR126463]
since its creation.
Description: This issue looked like a simple 1-character typo, but when trying to
construct a suitable testcase, I found that the patterns were hardly
ever being used. In the end there turned out to be three underlying
issues:
- we were selecting the wrong CCmode for NEG and MINUS overflow
checks (more precisely, we weren't detecting these at all and
then falling back to CCmode).
- The patterns were not in canonical form, preventing combine
from finding a match while optimizing.
- Finally, when we did eventually get the preferred form, we
had the invalid output template, causing the reported ICE.
gcc/ChangeLog:
PR target/126463
* config/aarch64/aarch64.cc (aarch64_select_cc_mode): Correctly
match overflow detection idioms using MINUS and NEG.
* config/aarch64/aarch64.md (subv<GPI:mode>4): Re-order the
compare into canonical form.
(subv<GPI:mode>4_insn): Likewise.
(negv<GPI:mode>4): Likewise.
(negv<GPI:mode>_insn): Likewise. Fix typo in output template.
gcc/testsuite/ChangeLog:
PR target/126463
* gcc.target/aarch64/negv.c: New test.
The full and up to date discussion can be found at https://forge.sourceware.org/gcc/gcc/pulls/227
The merge request has been closed without being merged directly on the forge repository.
On 2026-09-04 16:19:19+00:00, Wilco Dijkstra (Wilco) <wilco.dijkstra@arm.com> approved the changes:
LGTM. OK
On 2026-09-04 17:01:33+00:00, Richard Earnshaw (rearnsha) wrote:
Thanks, committed.
@@ -12792,11 +12792,13 @@ aarch64_select_cc_mode (RTX_CODE code, rtx x, rtx y)
<< (GET_MODE_BITSIZE (mode_x).to_constant () / 2))))
return CC_ADCmode;
- /* A test for signed overflow. */
+ /* Tests for signed overflow. */
if ((mode_x == DImode || mode_x == TImode)
- && code == NE
- && code_x == PLUS
- && GET_CODE (y) == SIGN_EXTEND)
+ && (code == NE || code == EQ)
+ && (code_x == PLUS || code_x == MINUS || code_x == NEG)
+ && GET_CODE (XEXP (x, 0)) == SIGN_EXTEND
+ && GET_CODE (y) == SIGN_EXTEND
+ && GET_CODE (XEXP (y, 0)) == code_x)
return CC_Vmode;
/* For everything else, return CCmode. */
@@ -3598,12 +3598,11 @@
(define_insn "subv<GPI:mode>_insn"
[(set (reg:CC_V CC_REGNUM)
(compare:CC_V
+ (minus:<DWI>
+ (sign_extend:<DWI> (match_operand:GPI 1 "register_operand" "rk"))
+ (sign_extend:<DWI> (match_operand:GPI 2 "register_operand" "r")))
(sign_extend:<DWI>
- (minus:GPI
- (match_operand:GPI 1 "register_operand" "rk")
- (match_operand:GPI 2 "register_operand" "r")))
- (minus:<DWI> (sign_extend:<DWI> (match_dup 1))
- (sign_extend:<DWI> (match_dup 2)))))
+ (minus:GPI (match_dup 1) (match_dup 2)))))
(set (match_operand:GPI 0 "register_operand" "=r")
(minus:GPI (match_dup 1) (match_dup 2)))]
""
@@ -3614,12 +3613,11 @@
(define_insn "subv<GPI:mode>_imm"
[(set (reg:CC_V CC_REGNUM)
(compare:CC_V
+ (minus:<DWI>
+ (sign_extend:<DWI> (match_operand:GPI 1 "register_operand"))
+ (match_operand:GPI 2 "aarch64_plus_immediate"))
(sign_extend:<DWI>
- (minus:GPI
- (match_operand:GPI 1 "register_operand")
- (match_operand:GPI 2 "aarch64_plus_immediate")))
- (minus:<DWI> (sign_extend:<DWI> (match_dup 1))
- (match_dup 2))))
+ (minus:GPI (match_dup 1) (match_dup 2)))))
(set (match_operand:GPI 0 "register_operand")
(minus:GPI (match_dup 1) (match_dup 2)))]
""
@@ -3646,9 +3644,10 @@
(define_insn "negv<GPI:mode>_insn"
[(set (reg:CC_V CC_REGNUM)
(compare:CC_V
+ (neg:<DWI>
+ (sign_extend:<DWI> (match_operand:GPI 1 "register_operand" "r")))
(sign_extend:<DWI>
- (neg:GPI (match_operand:GPI 1 "register_operand" "r")))
- (neg:<DWI> (sign_extend:<DWI> (match_dup 1)))))
+ (neg:GPI (match_dup 1)))))
(set (match_operand:GPI 0 "register_operand" "=r")
(neg:GPI (match_dup 1)))]
""
@@ -3659,22 +3658,23 @@
(define_insn "negv<GPI:mode>_cmp_only"
[(set (reg:CC_V CC_REGNUM)
(compare:CC_V
+ (neg:<DWI>
+ (sign_extend:<DWI> (match_operand:GPI 0 "register_operand" "r")))
(sign_extend:<DWI>
- (neg:GPI (match_operand:GPI 0 "register_operand" "r")))
- (neg:<DWI> (sign_extend:<DWI> (match_dup 0)))))]
+ (neg:GPI (match_dup 0)))))]
""
- "negs\\t%<w>zr, %<w>0"
+ "negs\\t<w>zr, %<w>0"
[(set_attr "type" "alus_sreg")]
)
(define_insn "*cmpv<GPI:mode>_insn"
[(set (reg:CC_V CC_REGNUM)
(compare:CC_V
+ (minus:<DWI>
+ (sign_extend:<DWI> (match_operand:GPI 0 "register_operand"))
+ (sign_extend:<DWI> (match_operand:GPI 1 "aarch64_plus_operand")))
(sign_extend:<DWI>
- (minus:GPI (match_operand:GPI 0 "register_operand")
- (match_operand:GPI 1 "aarch64_plus_operand")))
- (minus:<DWI> (sign_extend:<DWI> (match_dup 0))
- (sign_extend:<DWI> (match_dup 1)))))]
+ (minus:GPI (match_dup 0) (match_dup 1)))))]
""
{@ [ cons: 0 , 1 ]
[ r , r ] cmp\t%<w>0, %<w>1
new file mode 100644
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-O" } */
+/* { dg-final { check-function-bodies "**" "" "" } } */
+
+_Bool negvsi (int x)
+{
+ int result;
+ return __builtin_sub_overflow (0, x, &result);
+}
+/*
+** negvsi:
+** negs wzr, w0
+** cset w0, vs
+** ret
+*/
+
+_Bool negvdi (long x)
+{
+ long result;
+ return __builtin_sub_overflow (0, x, &result);
+}
+/*
+** negvdi:
+** negs xzr, x0
+** cset w0, vs
+** ret
+*/