* [PATCH] lower-bitint: Fix up -fnon-call-exceptions bit-field load lowering [PR112668]
@ 2023-11-23 9:42 Jakub Jelinek
2023-11-23 10:01 ` Richard Biener
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2023-11-23 9:42 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
Hi!
As the following testcase shows, there are some bugs in the
-fnon-call-exceptions bit-field load lowering. In particular, there
is a case where we want to emit a load early in the initialization
(before m_init_gsi) and because that load might throw exception, need
to split block after the load so that it has an EH edge.
Now, across this splitting, we have m_init_gsi, save_gsi (something
we put back into m_gsi afterwards) statement iterators and m_preheader_bb
which is used to determine the pre-header edge of a loop (if any).
As the testcase shows, both of these statement iterators and m_preheader_bb
as well need adjustments if the block was split. If the stmt iterators
refer to a statement, they need to be updated so that if the statement is
in the bb after the split gsi_bb and gsi_seq is updated, otherwise they
ought to be the start of the new (second) bb.
Similarly, m_preheader_bb should be updated to the second bb if it was
the first before. Other spots where we insert something before m_init_gsi
don't split blocks in there and are fine.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2023-11-23 Jakub Jelinek <jakub@redhat.com>
PR middle-end/112668
* gimple-lower-bitint.cc (bitint_large_huge::handle_load): When
splitting gsi_bb (m_init_gsi) basic block, update m_preheader_bb
if needed, fix up update of m_init_gsi and update saved m_gsi
as well if needed.
* gcc.dg/bitint-40.c: New test.
--- gcc/gimple-lower-bitint.cc.jj 2023-11-14 10:52:16.000000000 +0100
+++ gcc/gimple-lower-bitint.cc 2023-11-22 14:34:17.327140002 +0100
@@ -1687,7 +1687,22 @@ bitint_large_huge::handle_load (gimple *
edge e = split_block (gsi_bb (m_gsi), g);
make_edge (e->src, eh_edge->dest, EDGE_EH)->probability
= profile_probability::very_unlikely ();
- m_init_gsi.bb = e->dest;
+ m_init_gsi = gsi_last_bb (e->dest);
+ if (!gsi_end_p (m_init_gsi))
+ gsi_next (&m_init_gsi);
+ if (gsi_bb (save_gsi) == e->src)
+ {
+ if (gsi_end_p (save_gsi))
+ {
+ save_gsi = gsi_last_bb (e->dest);
+ if (!gsi_end_p (save_gsi))
+ gsi_next (&save_gsi);
+ }
+ else
+ save_gsi = gsi_for_stmt (gsi_stmt (save_gsi));
+ }
+ if (m_preheader_bb == e->src)
+ m_preheader_bb = e->dest;
}
}
m_gsi = save_gsi;
--- gcc/testsuite/gcc.dg/bitint-40.c.jj 2023-11-22 13:47:12.380580107 +0100
+++ gcc/testsuite/gcc.dg/bitint-40.c 2023-11-22 14:35:50.225842768 +0100
@@ -0,0 +1,29 @@
+/* PR middle-end/112668 */
+/* { dg-do compile { target bitint } } */
+/* { dg-options "-std=c23 -fnon-call-exceptions" } */
+
+#if __BITINT_MAXWIDTH__ >= 156
+struct T156 { _BitInt(156) a : 2; unsigned _BitInt(156) b : 135; _BitInt(156) c : 2; };
+extern void foo156 (struct T156 *);
+
+unsigned _BitInt(156)
+bar156 (int i)
+{
+ struct T156 r156[12];
+ foo156 (&r156[0]);
+ return r156[i].b;
+}
+#endif
+
+#if __BITINT_MAXWIDTH__ >= 495
+struct T495 { _BitInt(495) a : 2; unsigned _BitInt(495) b : 471; _BitInt(495) c : 2; };
+extern void foo495 (struct T495 *r495);
+
+unsigned _BitInt(495)
+bar495 (int i)
+{
+ struct T495 r495[12];
+ foo495 (r495);
+ return r495[i].b;
+}
+#endif
Jakub
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] lower-bitint: Fix up -fnon-call-exceptions bit-field load lowering [PR112668]
2023-11-23 9:42 [PATCH] lower-bitint: Fix up -fnon-call-exceptions bit-field load lowering [PR112668] Jakub Jelinek
@ 2023-11-23 10:01 ` Richard Biener
2023-11-23 10:56 ` Jakub Jelinek
0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2023-11-23 10:01 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches
On Thu, Nov 23, 2023 at 10:43 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> As the following testcase shows, there are some bugs in the
> -fnon-call-exceptions bit-field load lowering. In particular, there
> is a case where we want to emit a load early in the initialization
> (before m_init_gsi) and because that load might throw exception, need
> to split block after the load so that it has an EH edge.
> Now, across this splitting, we have m_init_gsi, save_gsi (something
> we put back into m_gsi afterwards) statement iterators and m_preheader_bb
> which is used to determine the pre-header edge of a loop (if any).
> As the testcase shows, both of these statement iterators and m_preheader_bb
> as well need adjustments if the block was split. If the stmt iterators
> refer to a statement, they need to be updated so that if the statement is
> in the bb after the split gsi_bb and gsi_seq is updated, otherwise they
> ought to be the start of the new (second) bb.
> Similarly, m_preheader_bb should be updated to the second bb if it was
> the first before. Other spots where we insert something before m_init_gsi
> don't split blocks in there and are fine.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Wheee ...
> 2023-11-23 Jakub Jelinek <jakub@redhat.com>
>
> PR middle-end/112668
> * gimple-lower-bitint.cc (bitint_large_huge::handle_load): When
> splitting gsi_bb (m_init_gsi) basic block, update m_preheader_bb
> if needed, fix up update of m_init_gsi and update saved m_gsi
> as well if needed.
>
> * gcc.dg/bitint-40.c: New test.
>
> --- gcc/gimple-lower-bitint.cc.jj 2023-11-14 10:52:16.000000000 +0100
> +++ gcc/gimple-lower-bitint.cc 2023-11-22 14:34:17.327140002 +0100
> @@ -1687,7 +1687,22 @@ bitint_large_huge::handle_load (gimple *
> edge e = split_block (gsi_bb (m_gsi), g);
> make_edge (e->src, eh_edge->dest, EDGE_EH)->probability
> = profile_probability::very_unlikely ();
> - m_init_gsi.bb = e->dest;
> + m_init_gsi = gsi_last_bb (e->dest);
shouldn't that be gsi_start_bb (e->dest) if we want to continue inserting
before the "old" stmt?
> + if (!gsi_end_p (m_init_gsi))
> + gsi_next (&m_init_gsi);
That would always put it at the end?
> + if (gsi_bb (save_gsi) == e->src)
> + {
> + if (gsi_end_p (save_gsi))
> + {
> + save_gsi = gsi_last_bb (e->dest);
> + if (!gsi_end_p (save_gsi))
> + gsi_next (&save_gsi);
> + }
> + else
> + save_gsi = gsi_for_stmt (gsi_stmt (save_gsi));
uhm. It might be better to instead of doing save_gsi = m_gsi
save gsi_stmt () and gsi_bb () to avoid accessing the now
possibly invalid iterator?
If there were only one iterator I'd say we want a
split_block_{after,before} (&gsi);
which hides the detail of updating the iterator. But you have the
additional issue of possibly updating another iterator where as said
the better solution would be to reconstruct it from a gimple *
(or basic_block if at end). Maybe we can have a
gsi_update_after_spli_block (&gsi, basic_block-that-was-split)?
If you think any of this would be an improvement (but also see
the gsi_last_bb vs gsi_start issue) feel free to improve.
Otherwise OK as-is.
Richard.
> + }
> + if (m_preheader_bb == e->src)
> + m_preheader_bb = e->dest;
> }
> }
> m_gsi = save_gsi;
> --- gcc/testsuite/gcc.dg/bitint-40.c.jj 2023-11-22 13:47:12.380580107 +0100
> +++ gcc/testsuite/gcc.dg/bitint-40.c 2023-11-22 14:35:50.225842768 +0100
> @@ -0,0 +1,29 @@
> +/* PR middle-end/112668 */
> +/* { dg-do compile { target bitint } } */
> +/* { dg-options "-std=c23 -fnon-call-exceptions" } */
> +
> +#if __BITINT_MAXWIDTH__ >= 156
> +struct T156 { _BitInt(156) a : 2; unsigned _BitInt(156) b : 135; _BitInt(156) c : 2; };
> +extern void foo156 (struct T156 *);
> +
> +unsigned _BitInt(156)
> +bar156 (int i)
> +{
> + struct T156 r156[12];
> + foo156 (&r156[0]);
> + return r156[i].b;
> +}
> +#endif
> +
> +#if __BITINT_MAXWIDTH__ >= 495
> +struct T495 { _BitInt(495) a : 2; unsigned _BitInt(495) b : 471; _BitInt(495) c : 2; };
> +extern void foo495 (struct T495 *r495);
> +
> +unsigned _BitInt(495)
> +bar495 (int i)
> +{
> + struct T495 r495[12];
> + foo495 (r495);
> + return r495[i].b;
> +}
> +#endif
>
> Jakub
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] lower-bitint: Fix up -fnon-call-exceptions bit-field load lowering [PR112668]
2023-11-23 10:01 ` Richard Biener
@ 2023-11-23 10:56 ` Jakub Jelinek
2023-11-23 11:54 ` Jakub Jelinek
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2023-11-23 10:56 UTC (permalink / raw)
To: Richard Biener; +Cc: Richard Biener, gcc-patches
On Thu, Nov 23, 2023 at 11:01:08AM +0100, Richard Biener wrote:
> > --- gcc/gimple-lower-bitint.cc.jj 2023-11-14 10:52:16.000000000 +0100
> > +++ gcc/gimple-lower-bitint.cc 2023-11-22 14:34:17.327140002 +0100
> > @@ -1687,7 +1687,22 @@ bitint_large_huge::handle_load (gimple *
> > edge e = split_block (gsi_bb (m_gsi), g);
> > make_edge (e->src, eh_edge->dest, EDGE_EH)->probability
> > = profile_probability::very_unlikely ();
> > - m_init_gsi.bb = e->dest;
> > + m_init_gsi = gsi_last_bb (e->dest);
>
> shouldn't that be gsi_start_bb (e->dest) if we want to continue inserting
> before the "old" stmt?
>
> > + if (!gsi_end_p (m_init_gsi))
> > + gsi_next (&m_init_gsi);
>
> That would always put it at the end?
>
> > + if (gsi_bb (save_gsi) == e->src)
> > + {
> > + if (gsi_end_p (save_gsi))
> > + {
> > + save_gsi = gsi_last_bb (e->dest);
> > + if (!gsi_end_p (save_gsi))
> > + gsi_next (&save_gsi);
> > + }
> > + else
> > + save_gsi = gsi_for_stmt (gsi_stmt (save_gsi));
>
> uhm. It might be better to instead of doing save_gsi = m_gsi
> save gsi_stmt () and gsi_bb () to avoid accessing the now
> possibly invalid iterator?
>
> If there were only one iterator I'd say we want a
>
> split_block_{after,before} (&gsi);
>
> which hides the detail of updating the iterator. But you have the
> additional issue of possibly updating another iterator where as said
> the better solution would be to reconstruct it from a gimple *
> (or basic_block if at end). Maybe we can have a
> gsi_update_after_spli_block (&gsi, basic_block-that-was-split)?
>
> If you think any of this would be an improvement (but also see
> the gsi_last_bb vs gsi_start issue) feel free to improve.
>
> Otherwise OK as-is.
The code has 2 iterators and pretty much everything in the pass inserts
statements before iterator.
m_gsi is the iterator before which everything is inserted, initially
initialized to
m_gsi = gsi_for_stmt (stmt);
where stmt is the statement we are lowering, but updated in many places
when splitting a bb etc. So, it needs to behave right for the insert
before behavior, gsi_end_p means insert at the end of bb.
Then m_init_gsi is initially one statement earlier, so insert code
after that statement instead:
m_init_gsi = m_gsi;
gsi_prev (&m_init_gsi);
with gsi_end_p meaning insert at the start of bb.
Because all the pass infrastructure is for inserting before rather than
after, when inserting (temporarily) after m_init_gsi, it does
gimple_stmt_iterator save_gsi = m_gsi;
m_gsi = m_init_gsi;
if (gsi_end_p (m_gsi))
m_gsi = gsi_after_labels (gsi_bb (m_gsi));
else
gsi_next (&m_gsi);
and then it can insert before m_gsi and finally when done there
m_gsi = save_gsi;
The problematic splitting of the bb is during this temporary override
to insert stuff after m_init_gsi.
For the save_gsi update, I believe I'm reconstructing it from
the stmt if any and set to gsi_end_p if it was gsi_end_p before:
+ if (gsi_end_p (save_gsi))
+ {
+ save_gsi = gsi_last_bb (e->dest);
+ if (!gsi_end_p (save_gsi))
+ gsi_next (&save_gsi);
+ }
+ else
+ save_gsi = gsi_for_stmt (gsi_stmt (save_gsi));
I don't know how else to re-create a gsi_end_p iterator from a bb
(other option I know of would be gsi_start_bb and gsi_prev if !gsi_end_p,
but stmt after gsi_last_bb seems to match more the intent).
Now, regarding m_init_gsi, I think I'll need to play around, maybe
I should have in the end insert after and update behavior rather than
insert after, and that could be achieved by adding
m_init_gsi = m_gsi;
gsi_prev (&m_init_gsi);
before the
m_gsi = save_gsi;
restore in all the 3 places and then no other updates of m_init_gsi would be
needed. Except gsi_prev likely won't like the gsi_end_p (m_gsi) case,
so maybe
m_init_gsi = m_gsi;
if (gsi_end_p (m_init_gsi))
m_init_gsi = gsi_last_bb (gsi_bb (m_init_gsi));
else
gsi_prev (&m_init_gsi);
m_gsi = save_gsi;
Jakub
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] lower-bitint: Fix up -fnon-call-exceptions bit-field load lowering [PR112668]
2023-11-23 10:56 ` Jakub Jelinek
@ 2023-11-23 11:54 ` Jakub Jelinek
2023-11-23 12:10 ` Richard Biener
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2023-11-23 11:54 UTC (permalink / raw)
To: Richard Biener, Richard Biener, gcc-patches
On Thu, Nov 23, 2023 at 11:56:33AM +0100, Jakub Jelinek wrote:
> Now, regarding m_init_gsi, I think I'll need to play around, maybe
> I should have in the end insert after and update behavior rather than
> insert after, and that could be achieved by adding
> m_init_gsi = m_gsi;
> gsi_prev (&m_init_gsi);
> before the
> m_gsi = save_gsi;
> restore in all the 3 places and then no other updates of m_init_gsi would be
> needed. Except gsi_prev likely won't like the gsi_end_p (m_gsi) case,
> so maybe
> m_init_gsi = m_gsi;
> if (gsi_end_p (m_init_gsi))
> m_init_gsi = gsi_last_bb (gsi_bb (m_init_gsi));
> else
> gsi_prev (&m_init_gsi);
> m_gsi = save_gsi;
That seems to work and I think it is the right thing to do.
Here is an updated patch to do that. Passed
make check-gcc -j32 -k GCC_TEST_RUN_EXPENSIVE=1 RUNTESTFLAGS='GCC_TEST_RUN_EXPENSIVE=1 dg.exp=*bitint* dg-torture.exp=*bitint*'
so far.
2023-11-23 Jakub Jelinek <jakub@redhat.com>
PR middle-end/112668
* gimple-lower-bitint.cc (bitint_large_huge::handle_cast): After
temporarily adding statements after m_init_gsi, update m_init_gsi
such that later additions after it will be after the added statements.
(bitint_large_huge::handle_load): Likewise. When splitting
gsi_bb (m_init_gsi) basic block, update m_preheader_bb if needed
and update saved m_gsi as well if needed.
* gcc.dg/bitint-40.c: New test.
--- gcc/gimple-lower-bitint.cc.jj 2023-11-22 22:55:14.260164718 +0100
+++ gcc/gimple-lower-bitint.cc 2023-11-23 12:39:35.030364243 +0100
@@ -1294,6 +1294,11 @@ bitint_large_huge::handle_cast (tree lhs
g = gimple_build_assign (n, RSHIFT_EXPR, t, lpm1);
insert_before (g);
m_data[save_data_cnt + 1] = add_cast (m_limb_type, n);
+ m_init_gsi = m_gsi;
+ if (gsi_end_p (m_init_gsi))
+ m_init_gsi = gsi_last_bb (gsi_bb (m_init_gsi));
+ else
+ gsi_prev (&m_init_gsi);
m_gsi = save_gsi;
}
else if (m_upwards_2limb * limb_prec < TYPE_PRECISION (rhs_type))
@@ -1523,6 +1528,11 @@ bitint_large_huge::handle_cast (tree lhs
insert_before (g);
rext = add_cast (m_limb_type, gimple_assign_lhs (g));
}
+ m_init_gsi = m_gsi;
+ if (gsi_end_p (m_init_gsi))
+ m_init_gsi = gsi_last_bb (gsi_bb (m_init_gsi));
+ else
+ gsi_prev (&m_init_gsi);
m_gsi = save_gsi;
}
tree t;
@@ -1687,9 +1697,27 @@ bitint_large_huge::handle_load (gimple *
edge e = split_block (gsi_bb (m_gsi), g);
make_edge (e->src, eh_edge->dest, EDGE_EH)->probability
= profile_probability::very_unlikely ();
- m_init_gsi.bb = e->dest;
+ m_gsi = gsi_after_labels (e->dest);
+ if (gsi_bb (save_gsi) == e->src)
+ {
+ if (gsi_end_p (save_gsi))
+ {
+ save_gsi = gsi_last_bb (e->dest);
+ if (!gsi_end_p (save_gsi))
+ gsi_next (&save_gsi);
+ }
+ else
+ save_gsi = gsi_for_stmt (gsi_stmt (save_gsi));
+ }
+ if (m_preheader_bb == e->src)
+ m_preheader_bb = e->dest;
}
}
+ m_init_gsi = m_gsi;
+ if (gsi_end_p (m_init_gsi))
+ m_init_gsi = gsi_last_bb (gsi_bb (m_init_gsi));
+ else
+ gsi_prev (&m_init_gsi);
m_gsi = save_gsi;
tree out;
prepare_data_in_out (iv, idx, &out);
--- gcc/testsuite/gcc.dg/bitint-40.c.jj 2023-11-22 13:47:12.380580107 +0100
+++ gcc/testsuite/gcc.dg/bitint-40.c 2023-11-22 14:35:50.225842768 +0100
@@ -0,0 +1,29 @@
+/* PR middle-end/112668 */
+/* { dg-do compile { target bitint } } */
+/* { dg-options "-std=c23 -fnon-call-exceptions" } */
+
+#if __BITINT_MAXWIDTH__ >= 156
+struct T156 { _BitInt(156) a : 2; unsigned _BitInt(156) b : 135; _BitInt(156) c : 2; };
+extern void foo156 (struct T156 *);
+
+unsigned _BitInt(156)
+bar156 (int i)
+{
+ struct T156 r156[12];
+ foo156 (&r156[0]);
+ return r156[i].b;
+}
+#endif
+
+#if __BITINT_MAXWIDTH__ >= 495
+struct T495 { _BitInt(495) a : 2; unsigned _BitInt(495) b : 471; _BitInt(495) c : 2; };
+extern void foo495 (struct T495 *r495);
+
+unsigned _BitInt(495)
+bar495 (int i)
+{
+ struct T495 r495[12];
+ foo495 (r495);
+ return r495[i].b;
+}
+#endif
Jakub
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] lower-bitint: Fix up -fnon-call-exceptions bit-field load lowering [PR112668]
2023-11-23 11:54 ` Jakub Jelinek
@ 2023-11-23 12:10 ` Richard Biener
2023-11-23 13:56 ` [PATCH] lower-bitint, v3: " Jakub Jelinek
0 siblings, 1 reply; 7+ messages in thread
From: Richard Biener @ 2023-11-23 12:10 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches
On Thu, Nov 23, 2023 at 12:55 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Thu, Nov 23, 2023 at 11:56:33AM +0100, Jakub Jelinek wrote:
> > Now, regarding m_init_gsi, I think I'll need to play around, maybe
> > I should have in the end insert after and update behavior rather than
> > insert after, and that could be achieved by adding
> > m_init_gsi = m_gsi;
> > gsi_prev (&m_init_gsi);
> > before the
> > m_gsi = save_gsi;
> > restore in all the 3 places and then no other updates of m_init_gsi would be
> > needed. Except gsi_prev likely won't like the gsi_end_p (m_gsi) case,
> > so maybe
> > m_init_gsi = m_gsi;
> > if (gsi_end_p (m_init_gsi))
> > m_init_gsi = gsi_last_bb (gsi_bb (m_init_gsi));
> > else
> > gsi_prev (&m_init_gsi);
> > m_gsi = save_gsi;
>
> That seems to work and I think it is the right thing to do.
>
> Here is an updated patch to do that. Passed
> make check-gcc -j32 -k GCC_TEST_RUN_EXPENSIVE=1 RUNTESTFLAGS='GCC_TEST_RUN_EXPENSIVE=1 dg.exp=*bitint* dg-torture.exp=*bitint*'
> so far.
Looks a bit better. As for constructing a gsi_end_p () iterator for a
basic-block
I'd simply add a new gsi_end_{bb,seq} ({basic_block,gimple_seq}).
Richard.
> 2023-11-23 Jakub Jelinek <jakub@redhat.com>
>
> PR middle-end/112668
> * gimple-lower-bitint.cc (bitint_large_huge::handle_cast): After
> temporarily adding statements after m_init_gsi, update m_init_gsi
> such that later additions after it will be after the added statements.
> (bitint_large_huge::handle_load): Likewise. When splitting
> gsi_bb (m_init_gsi) basic block, update m_preheader_bb if needed
> and update saved m_gsi as well if needed.
>
> * gcc.dg/bitint-40.c: New test.
>
> --- gcc/gimple-lower-bitint.cc.jj 2023-11-22 22:55:14.260164718 +0100
> +++ gcc/gimple-lower-bitint.cc 2023-11-23 12:39:35.030364243 +0100
> @@ -1294,6 +1294,11 @@ bitint_large_huge::handle_cast (tree lhs
> g = gimple_build_assign (n, RSHIFT_EXPR, t, lpm1);
> insert_before (g);
> m_data[save_data_cnt + 1] = add_cast (m_limb_type, n);
> + m_init_gsi = m_gsi;
> + if (gsi_end_p (m_init_gsi))
> + m_init_gsi = gsi_last_bb (gsi_bb (m_init_gsi));
> + else
> + gsi_prev (&m_init_gsi);
> m_gsi = save_gsi;
> }
> else if (m_upwards_2limb * limb_prec < TYPE_PRECISION (rhs_type))
> @@ -1523,6 +1528,11 @@ bitint_large_huge::handle_cast (tree lhs
> insert_before (g);
> rext = add_cast (m_limb_type, gimple_assign_lhs (g));
> }
> + m_init_gsi = m_gsi;
> + if (gsi_end_p (m_init_gsi))
> + m_init_gsi = gsi_last_bb (gsi_bb (m_init_gsi));
> + else
> + gsi_prev (&m_init_gsi);
> m_gsi = save_gsi;
> }
> tree t;
> @@ -1687,9 +1697,27 @@ bitint_large_huge::handle_load (gimple *
> edge e = split_block (gsi_bb (m_gsi), g);
> make_edge (e->src, eh_edge->dest, EDGE_EH)->probability
> = profile_probability::very_unlikely ();
> - m_init_gsi.bb = e->dest;
> + m_gsi = gsi_after_labels (e->dest);
> + if (gsi_bb (save_gsi) == e->src)
> + {
> + if (gsi_end_p (save_gsi))
> + {
> + save_gsi = gsi_last_bb (e->dest);
> + if (!gsi_end_p (save_gsi))
> + gsi_next (&save_gsi);
> + }
> + else
> + save_gsi = gsi_for_stmt (gsi_stmt (save_gsi));
> + }
> + if (m_preheader_bb == e->src)
> + m_preheader_bb = e->dest;
> }
> }
> + m_init_gsi = m_gsi;
> + if (gsi_end_p (m_init_gsi))
> + m_init_gsi = gsi_last_bb (gsi_bb (m_init_gsi));
> + else
> + gsi_prev (&m_init_gsi);
> m_gsi = save_gsi;
> tree out;
> prepare_data_in_out (iv, idx, &out);
> --- gcc/testsuite/gcc.dg/bitint-40.c.jj 2023-11-22 13:47:12.380580107 +0100
> +++ gcc/testsuite/gcc.dg/bitint-40.c 2023-11-22 14:35:50.225842768 +0100
> @@ -0,0 +1,29 @@
> +/* PR middle-end/112668 */
> +/* { dg-do compile { target bitint } } */
> +/* { dg-options "-std=c23 -fnon-call-exceptions" } */
> +
> +#if __BITINT_MAXWIDTH__ >= 156
> +struct T156 { _BitInt(156) a : 2; unsigned _BitInt(156) b : 135; _BitInt(156) c : 2; };
> +extern void foo156 (struct T156 *);
> +
> +unsigned _BitInt(156)
> +bar156 (int i)
> +{
> + struct T156 r156[12];
> + foo156 (&r156[0]);
> + return r156[i].b;
> +}
> +#endif
> +
> +#if __BITINT_MAXWIDTH__ >= 495
> +struct T495 { _BitInt(495) a : 2; unsigned _BitInt(495) b : 471; _BitInt(495) c : 2; };
> +extern void foo495 (struct T495 *r495);
> +
> +unsigned _BitInt(495)
> +bar495 (int i)
> +{
> + struct T495 r495[12];
> + foo495 (r495);
> + return r495[i].b;
> +}
> +#endif
>
>
> Jakub
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH] lower-bitint, v3: Fix up -fnon-call-exceptions bit-field load lowering [PR112668]
2023-11-23 12:10 ` Richard Biener
@ 2023-11-23 13:56 ` Jakub Jelinek
2023-11-23 14:22 ` Richard Biener
0 siblings, 1 reply; 7+ messages in thread
From: Jakub Jelinek @ 2023-11-23 13:56 UTC (permalink / raw)
To: Richard Biener; +Cc: Richard Biener, gcc-patches
On Thu, Nov 23, 2023 at 01:10:02PM +0100, Richard Biener wrote:
> Looks a bit better. As for constructing a gsi_end_p () iterator for a
> basic-block
> I'd simply add a new gsi_end_{bb,seq} ({basic_block,gimple_seq}).
Ok, here it is (just used gsi_end without _seq suffix for gimple_seq &
because it is then consistent with gsi_start/gsi_last etc.).
2023-11-23 Jakub Jelinek <jakub@redhat.com>
PR middle-end/112668
* gimple-iterator.h (gsi_end, gsi_end_bb): New inline functions.
* gimple-lower-bitint.cc (bitint_large_huge::handle_cast): After
temporarily adding statements after m_init_gsi, update m_init_gsi
such that later additions after it will be after the added statements.
(bitint_large_huge::handle_load): Likewise. When splitting
gsi_bb (m_init_gsi) basic block, update m_preheader_bb if needed
and update saved m_gsi as well if needed.
(bitint_large_huge::lower_mergeable_stmt,
bitint_large_huge::lower_comparison_stmt,
bitint_large_huge::lower_mul_overflow,
bitint_large_huge::lower_bit_query): Use gsi_end_bb.
* gcc.dg/bitint-40.c: New test.
--- gcc/gimple-iterator.h.jj 2023-04-22 10:23:40.628612517 +0200
+++ gcc/gimple-iterator.h 2023-11-23 14:46:28.371861488 +0100
@@ -169,6 +169,41 @@ gsi_last_bb (basic_block bb)
return i;
}
+/* Return a new iterator pointing to before the first statement or after
+ last statement (depending on whether adding statements after it or before it)
+ in a GIMPLE_SEQ. */
+
+inline gimple_stmt_iterator
+gsi_end (gimple_seq &seq)
+{
+ gimple_stmt_iterator i;
+ gimple *g = gimple_seq_last (seq);
+
+ i.ptr = NULL;
+ i.seq = &seq;
+ i.bb = g ? gimple_bb (g) : NULL;
+
+ return i;
+}
+
+/* Return a new iterator pointing to before the first statement or after
+ last statement (depending on whether adding statements after it or before it)
+ in basic block BB. */
+
+inline gimple_stmt_iterator
+gsi_end_bb (basic_block bb)
+{
+ gimple_stmt_iterator i;
+ gimple_seq *seq;
+
+ seq = bb_seq_addr (bb);
+ i.ptr = NULL;
+ i.seq = seq;
+ i.bb = bb;
+
+ return i;
+}
+
/* Return true if I is at the end of its sequence. */
inline bool
--- gcc/gimple-lower-bitint.cc.jj 2023-11-23 12:55:16.967225422 +0100
+++ gcc/gimple-lower-bitint.cc 2023-11-23 14:30:02.830662509 +0100
@@ -1294,6 +1294,11 @@ bitint_large_huge::handle_cast (tree lhs
g = gimple_build_assign (n, RSHIFT_EXPR, t, lpm1);
insert_before (g);
m_data[save_data_cnt + 1] = add_cast (m_limb_type, n);
+ m_init_gsi = m_gsi;
+ if (gsi_end_p (m_init_gsi))
+ m_init_gsi = gsi_last_bb (gsi_bb (m_init_gsi));
+ else
+ gsi_prev (&m_init_gsi);
m_gsi = save_gsi;
}
else if (m_upwards_2limb * limb_prec < TYPE_PRECISION (rhs_type))
@@ -1523,6 +1528,11 @@ bitint_large_huge::handle_cast (tree lhs
insert_before (g);
rext = add_cast (m_limb_type, gimple_assign_lhs (g));
}
+ m_init_gsi = m_gsi;
+ if (gsi_end_p (m_init_gsi))
+ m_init_gsi = gsi_last_bb (gsi_bb (m_init_gsi));
+ else
+ gsi_prev (&m_init_gsi);
m_gsi = save_gsi;
}
tree t;
@@ -1687,9 +1697,23 @@ bitint_large_huge::handle_load (gimple *
edge e = split_block (gsi_bb (m_gsi), g);
make_edge (e->src, eh_edge->dest, EDGE_EH)->probability
= profile_probability::very_unlikely ();
- m_init_gsi.bb = e->dest;
+ m_gsi = gsi_after_labels (e->dest);
+ if (gsi_bb (save_gsi) == e->src)
+ {
+ if (gsi_end_p (save_gsi))
+ save_gsi = gsi_end_bb (e->dest);
+ else
+ save_gsi = gsi_for_stmt (gsi_stmt (save_gsi));
+ }
+ if (m_preheader_bb == e->src)
+ m_preheader_bb = e->dest;
}
}
+ m_init_gsi = m_gsi;
+ if (gsi_end_p (m_init_gsi))
+ m_init_gsi = gsi_last_bb (gsi_bb (m_init_gsi));
+ else
+ gsi_prev (&m_init_gsi);
m_gsi = save_gsi;
tree out;
prepare_data_in_out (iv, idx, &out);
@@ -2359,11 +2383,7 @@ bitint_large_huge::lower_mergeable_stmt
edge e = split_block (gsi_bb (gsi), gsi_stmt (gsi));
edge_bb = e->src;
if (kind == bitint_prec_large)
- {
- m_gsi = gsi_last_bb (edge_bb);
- if (!gsi_end_p (m_gsi))
- gsi_next (&m_gsi);
- }
+ m_gsi = gsi_end_bb (edge_bb);
}
else
m_after_stmt = stmt;
@@ -2816,9 +2836,7 @@ bitint_large_huge::lower_comparison_stmt
gsi_prev (&gsi);
edge e = split_block (gsi_bb (gsi), gsi_stmt (gsi));
edge_bb = e->src;
- m_gsi = gsi_last_bb (edge_bb);
- if (!gsi_end_p (m_gsi))
- gsi_next (&m_gsi);
+ m_gsi = gsi_end_bb (edge_bb);
edge *edges = XALLOCAVEC (edge, cnt * 2);
for (unsigned i = 0; i < cnt; i++)
@@ -4288,9 +4306,7 @@ bitint_large_huge::lower_mul_overflow (t
gsi_prev (&gsi);
edge e = split_block (gsi_bb (gsi), gsi_stmt (gsi));
edge_bb = e->src;
- m_gsi = gsi_last_bb (edge_bb);
- if (!gsi_end_p (m_gsi))
- gsi_next (&m_gsi);
+ m_gsi = gsi_end_bb (edge_bb);
tree cmp = build_zero_cst (m_limb_type);
for (unsigned i = 0; i < cnt; i++)
@@ -4560,11 +4576,7 @@ bitint_large_huge::lower_bit_query (gimp
edge e = split_block (gsi_bb (gsi), gsi_stmt (gsi));
edge_bb = e->src;
if (kind == bitint_prec_large)
- {
- m_gsi = gsi_last_bb (edge_bb);
- if (!gsi_end_p (m_gsi))
- gsi_next (&m_gsi);
- }
+ m_gsi = gsi_end_bb (edge_bb);
bqp = XALLOCAVEC (struct bq_details, cnt);
}
else
@@ -4717,9 +4729,7 @@ bitint_large_huge::lower_bit_query (gimp
gsi_prev (&gsi);
edge e = split_block (gsi_bb (gsi), gsi_stmt (gsi));
edge_bb = e->src;
- m_gsi = gsi_last_bb (edge_bb);
- if (!gsi_end_p (m_gsi))
- gsi_next (&m_gsi);
+ m_gsi = gsi_end_bb (edge_bb);
if (ifn == IFN_CLZ)
bqp = XALLOCAVEC (struct bq_details, cnt);
--- gcc/testsuite/gcc.dg/bitint-40.c.jj 2023-11-23 14:22:48.328769734 +0100
+++ gcc/testsuite/gcc.dg/bitint-40.c 2023-11-23 14:22:48.328769734 +0100
@@ -0,0 +1,29 @@
+/* PR middle-end/112668 */
+/* { dg-do compile { target bitint } } */
+/* { dg-options "-std=c23 -fnon-call-exceptions" } */
+
+#if __BITINT_MAXWIDTH__ >= 156
+struct T156 { _BitInt(156) a : 2; unsigned _BitInt(156) b : 135; _BitInt(156) c : 2; };
+extern void foo156 (struct T156 *);
+
+unsigned _BitInt(156)
+bar156 (int i)
+{
+ struct T156 r156[12];
+ foo156 (&r156[0]);
+ return r156[i].b;
+}
+#endif
+
+#if __BITINT_MAXWIDTH__ >= 495
+struct T495 { _BitInt(495) a : 2; unsigned _BitInt(495) b : 471; _BitInt(495) c : 2; };
+extern void foo495 (struct T495 *r495);
+
+unsigned _BitInt(495)
+bar495 (int i)
+{
+ struct T495 r495[12];
+ foo495 (r495);
+ return r495[i].b;
+}
+#endif
Jakub
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] lower-bitint, v3: Fix up -fnon-call-exceptions bit-field load lowering [PR112668]
2023-11-23 13:56 ` [PATCH] lower-bitint, v3: " Jakub Jelinek
@ 2023-11-23 14:22 ` Richard Biener
0 siblings, 0 replies; 7+ messages in thread
From: Richard Biener @ 2023-11-23 14:22 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches
On Thu, Nov 23, 2023 at 2:56 PM Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Thu, Nov 23, 2023 at 01:10:02PM +0100, Richard Biener wrote:
> > Looks a bit better. As for constructing a gsi_end_p () iterator for a
> > basic-block
> > I'd simply add a new gsi_end_{bb,seq} ({basic_block,gimple_seq}).
>
> Ok, here it is (just used gsi_end without _seq suffix for gimple_seq &
> because it is then consistent with gsi_start/gsi_last etc.).
OK. It occurs to me that start + last and now end is somewhat
inconsistent and it should have been first + last, but well ...
Richard.
> 2023-11-23 Jakub Jelinek <jakub@redhat.com>
>
> PR middle-end/112668
> * gimple-iterator.h (gsi_end, gsi_end_bb): New inline functions.
> * gimple-lower-bitint.cc (bitint_large_huge::handle_cast): After
> temporarily adding statements after m_init_gsi, update m_init_gsi
> such that later additions after it will be after the added statements.
> (bitint_large_huge::handle_load): Likewise. When splitting
> gsi_bb (m_init_gsi) basic block, update m_preheader_bb if needed
> and update saved m_gsi as well if needed.
> (bitint_large_huge::lower_mergeable_stmt,
> bitint_large_huge::lower_comparison_stmt,
> bitint_large_huge::lower_mul_overflow,
> bitint_large_huge::lower_bit_query): Use gsi_end_bb.
>
> * gcc.dg/bitint-40.c: New test.
>
> --- gcc/gimple-iterator.h.jj 2023-04-22 10:23:40.628612517 +0200
> +++ gcc/gimple-iterator.h 2023-11-23 14:46:28.371861488 +0100
> @@ -169,6 +169,41 @@ gsi_last_bb (basic_block bb)
> return i;
> }
>
> +/* Return a new iterator pointing to before the first statement or after
> + last statement (depending on whether adding statements after it or before it)
> + in a GIMPLE_SEQ. */
> +
> +inline gimple_stmt_iterator
> +gsi_end (gimple_seq &seq)
> +{
> + gimple_stmt_iterator i;
> + gimple *g = gimple_seq_last (seq);
> +
> + i.ptr = NULL;
> + i.seq = &seq;
> + i.bb = g ? gimple_bb (g) : NULL;
> +
> + return i;
> +}
> +
> +/* Return a new iterator pointing to before the first statement or after
> + last statement (depending on whether adding statements after it or before it)
> + in basic block BB. */
> +
> +inline gimple_stmt_iterator
> +gsi_end_bb (basic_block bb)
> +{
> + gimple_stmt_iterator i;
> + gimple_seq *seq;
> +
> + seq = bb_seq_addr (bb);
> + i.ptr = NULL;
> + i.seq = seq;
> + i.bb = bb;
> +
> + return i;
> +}
> +
> /* Return true if I is at the end of its sequence. */
>
> inline bool
> --- gcc/gimple-lower-bitint.cc.jj 2023-11-23 12:55:16.967225422 +0100
> +++ gcc/gimple-lower-bitint.cc 2023-11-23 14:30:02.830662509 +0100
> @@ -1294,6 +1294,11 @@ bitint_large_huge::handle_cast (tree lhs
> g = gimple_build_assign (n, RSHIFT_EXPR, t, lpm1);
> insert_before (g);
> m_data[save_data_cnt + 1] = add_cast (m_limb_type, n);
> + m_init_gsi = m_gsi;
> + if (gsi_end_p (m_init_gsi))
> + m_init_gsi = gsi_last_bb (gsi_bb (m_init_gsi));
> + else
> + gsi_prev (&m_init_gsi);
> m_gsi = save_gsi;
> }
> else if (m_upwards_2limb * limb_prec < TYPE_PRECISION (rhs_type))
> @@ -1523,6 +1528,11 @@ bitint_large_huge::handle_cast (tree lhs
> insert_before (g);
> rext = add_cast (m_limb_type, gimple_assign_lhs (g));
> }
> + m_init_gsi = m_gsi;
> + if (gsi_end_p (m_init_gsi))
> + m_init_gsi = gsi_last_bb (gsi_bb (m_init_gsi));
> + else
> + gsi_prev (&m_init_gsi);
> m_gsi = save_gsi;
> }
> tree t;
> @@ -1687,9 +1697,23 @@ bitint_large_huge::handle_load (gimple *
> edge e = split_block (gsi_bb (m_gsi), g);
> make_edge (e->src, eh_edge->dest, EDGE_EH)->probability
> = profile_probability::very_unlikely ();
> - m_init_gsi.bb = e->dest;
> + m_gsi = gsi_after_labels (e->dest);
> + if (gsi_bb (save_gsi) == e->src)
> + {
> + if (gsi_end_p (save_gsi))
> + save_gsi = gsi_end_bb (e->dest);
> + else
> + save_gsi = gsi_for_stmt (gsi_stmt (save_gsi));
> + }
> + if (m_preheader_bb == e->src)
> + m_preheader_bb = e->dest;
> }
> }
> + m_init_gsi = m_gsi;
> + if (gsi_end_p (m_init_gsi))
> + m_init_gsi = gsi_last_bb (gsi_bb (m_init_gsi));
> + else
> + gsi_prev (&m_init_gsi);
> m_gsi = save_gsi;
> tree out;
> prepare_data_in_out (iv, idx, &out);
> @@ -2359,11 +2383,7 @@ bitint_large_huge::lower_mergeable_stmt
> edge e = split_block (gsi_bb (gsi), gsi_stmt (gsi));
> edge_bb = e->src;
> if (kind == bitint_prec_large)
> - {
> - m_gsi = gsi_last_bb (edge_bb);
> - if (!gsi_end_p (m_gsi))
> - gsi_next (&m_gsi);
> - }
> + m_gsi = gsi_end_bb (edge_bb);
> }
> else
> m_after_stmt = stmt;
> @@ -2816,9 +2836,7 @@ bitint_large_huge::lower_comparison_stmt
> gsi_prev (&gsi);
> edge e = split_block (gsi_bb (gsi), gsi_stmt (gsi));
> edge_bb = e->src;
> - m_gsi = gsi_last_bb (edge_bb);
> - if (!gsi_end_p (m_gsi))
> - gsi_next (&m_gsi);
> + m_gsi = gsi_end_bb (edge_bb);
>
> edge *edges = XALLOCAVEC (edge, cnt * 2);
> for (unsigned i = 0; i < cnt; i++)
> @@ -4288,9 +4306,7 @@ bitint_large_huge::lower_mul_overflow (t
> gsi_prev (&gsi);
> edge e = split_block (gsi_bb (gsi), gsi_stmt (gsi));
> edge_bb = e->src;
> - m_gsi = gsi_last_bb (edge_bb);
> - if (!gsi_end_p (m_gsi))
> - gsi_next (&m_gsi);
> + m_gsi = gsi_end_bb (edge_bb);
>
> tree cmp = build_zero_cst (m_limb_type);
> for (unsigned i = 0; i < cnt; i++)
> @@ -4560,11 +4576,7 @@ bitint_large_huge::lower_bit_query (gimp
> edge e = split_block (gsi_bb (gsi), gsi_stmt (gsi));
> edge_bb = e->src;
> if (kind == bitint_prec_large)
> - {
> - m_gsi = gsi_last_bb (edge_bb);
> - if (!gsi_end_p (m_gsi))
> - gsi_next (&m_gsi);
> - }
> + m_gsi = gsi_end_bb (edge_bb);
> bqp = XALLOCAVEC (struct bq_details, cnt);
> }
> else
> @@ -4717,9 +4729,7 @@ bitint_large_huge::lower_bit_query (gimp
> gsi_prev (&gsi);
> edge e = split_block (gsi_bb (gsi), gsi_stmt (gsi));
> edge_bb = e->src;
> - m_gsi = gsi_last_bb (edge_bb);
> - if (!gsi_end_p (m_gsi))
> - gsi_next (&m_gsi);
> + m_gsi = gsi_end_bb (edge_bb);
>
> if (ifn == IFN_CLZ)
> bqp = XALLOCAVEC (struct bq_details, cnt);
> --- gcc/testsuite/gcc.dg/bitint-40.c.jj 2023-11-23 14:22:48.328769734 +0100
> +++ gcc/testsuite/gcc.dg/bitint-40.c 2023-11-23 14:22:48.328769734 +0100
> @@ -0,0 +1,29 @@
> +/* PR middle-end/112668 */
> +/* { dg-do compile { target bitint } } */
> +/* { dg-options "-std=c23 -fnon-call-exceptions" } */
> +
> +#if __BITINT_MAXWIDTH__ >= 156
> +struct T156 { _BitInt(156) a : 2; unsigned _BitInt(156) b : 135; _BitInt(156) c : 2; };
> +extern void foo156 (struct T156 *);
> +
> +unsigned _BitInt(156)
> +bar156 (int i)
> +{
> + struct T156 r156[12];
> + foo156 (&r156[0]);
> + return r156[i].b;
> +}
> +#endif
> +
> +#if __BITINT_MAXWIDTH__ >= 495
> +struct T495 { _BitInt(495) a : 2; unsigned _BitInt(495) b : 471; _BitInt(495) c : 2; };
> +extern void foo495 (struct T495 *r495);
> +
> +unsigned _BitInt(495)
> +bar495 (int i)
> +{
> + struct T495 r495[12];
> + foo495 (r495);
> + return r495[i].b;
> +}
> +#endif
>
> Jakub
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-11-23 14:25 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-11-23 9:42 [PATCH] lower-bitint: Fix up -fnon-call-exceptions bit-field load lowering [PR112668] Jakub Jelinek
2023-11-23 10:01 ` Richard Biener
2023-11-23 10:56 ` Jakub Jelinek
2023-11-23 11:54 ` Jakub Jelinek
2023-11-23 12:10 ` Richard Biener
2023-11-23 13:56 ` [PATCH] lower-bitint, v3: " Jakub Jelinek
2023-11-23 14:22 ` Richard Biener
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).