On Mon, 8 Sep 2025, Jakub Jelinek wrote:
> On Mon, Sep 08, 2025 at 02:38:48PM +0200, Richard Biener wrote:
> > When there's an asm goto in the latch of a loop we may not use
> > IP_END IVs since instantiating those would (need to) split the
> > latch edge which in turn invalidates IP_NORMAL position handling.
> > This is a revision of the PR107997 fix.
> >
> > Bootstrap and regtest running on x86_64-unknown-linux-gnu.
> >
> > PR tree-optimization/107997
> > PR tree-optimization/121844
> > * tree-ssa-loop-ivopts.cc (allow_ip_end_pos_p): Do not allow
> > IP_END for latches ending with a control stmt.
> > (create_new_iv): Do not split the latch edge, instead assert
> > that's not necessary.
>
> LGTM.
> > * gcc.dg/torture/pr121844.c: New testcase.
>
> > --- /dev/null
> > +++ b/gcc/testsuite/gcc.dg/torture/pr121844.c
> > @@ -0,0 +1,19 @@
> > +/* { dg-do compile } */
> > +
> > +#include <stdint.h>
>
> Why? Does it rely on the size of any of those types in any way?
> Can't you s/int32_t/int/ and s/int16_t/void/ and probably
> unsigned long long * for type of e?
I've used unsigned long, this reproduces the failure.
> > +
> > +typedef uint64_t a;
> > +int32_t b[] = {};
> > +int32_t **c;
> > +int16_t d() {
> > + a *e;
>
> This one is uninitialized, is that needed for the repro?
> Can't it be e.g. argument to the function instead?
Yes, both 'e' and 'f' can be made function args.
I'll push the following.
Richard.
>From 3d777d03bb8c22204de1457cb2dae6a22fb2ed7a Mon Sep 17 00:00:00 2001
From: Richard Biener <rguent...@suse.de>
Date: Mon, 8 Sep 2025 14:32:38 +0200
Subject: [PATCH] tree-optimization/121844 - IVOPTs and asm goto in latch
To: gcc-patches@gcc.gnu.org
When there's an asm goto in the latch of a loop we may not use
IP_END IVs since instantiating those would (need to) split the
latch edge which in turn invalidates IP_NORMAL position handling.
This is a revision of the PR107997 fix.
PR tree-optimization/107997
PR tree-optimization/121844
* tree-ssa-loop-ivopts.cc (allow_ip_end_pos_p): Do not allow
IP_END for latches ending with a control stmt.
(create_new_iv): Do not split the latch edge, instead assert
that's not necessary.
* gcc.dg/torture/pr121844.c: New testcase.
---
gcc/testsuite/gcc.dg/torture/pr121844.c | 16 ++++++++++++++++
gcc/tree-ssa-loop-ivopts.cc | 13 +++++++------
2 files changed, 23 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/torture/pr121844.c
diff --git a/gcc/testsuite/gcc.dg/torture/pr121844.c
b/gcc/testsuite/gcc.dg/torture/pr121844.c
new file mode 100644
index 00000000000..149d5eaab64
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/torture/pr121844.c
@@ -0,0 +1,16 @@
+/* { dg-do compile } */
+
+typedef unsigned long a;
+int b[] = {};
+int **c;
+short d(a *e, int f)
+{
+ *c = &f;
+ for (;;)
+ asm goto("" : : : : g);
+ for (; f; f--) {
+ asm goto("" : : : : g);
+ g:
+ *e ^= b[f + 1];
+ }
+}
diff --git a/gcc/tree-ssa-loop-ivopts.cc b/gcc/tree-ssa-loop-ivopts.cc
index 2fe2655220b..ba727adc808 100644
--- a/gcc/tree-ssa-loop-ivopts.cc
+++ b/gcc/tree-ssa-loop-ivopts.cc
@@ -3200,6 +3200,12 @@ add_candidate_1 (struct ivopts_data *data, tree base,
tree step, bool important,
static bool
allow_ip_end_pos_p (class loop *loop)
{
+ /* Do not allow IP_END when creating the IV would need to split the
+ latch edge as that makes all IP_NORMAL invalid. */
+ auto pos = gsi_last_bb (ip_end_pos (loop));
+ if (!gsi_end_p (pos) && stmt_ends_bb_p (*pos))
+ return false;
+
if (!ip_normal_pos (loop))
return true;
@@ -7222,12 +7228,7 @@ create_new_iv (struct ivopts_data *data, struct iv_cand
*cand)
case IP_END:
incr_pos = gsi_last_bb (ip_end_pos (data->current_loop));
after = true;
- if (!gsi_end_p (incr_pos) && stmt_ends_bb_p (gsi_stmt (incr_pos)))
- {
- edge e = find_edge (gsi_bb (incr_pos), data->current_loop->header);
- incr_pos = gsi_after_labels (split_edge (e));
- after = false;
- }
+ gcc_assert (gsi_end_p (incr_pos) || !stmt_ends_bb_p (*incr_pos));
break;
case IP_AFTER_USE:
--
2.43.0
