Re: [PATCH] bitint: Fix up lowering of bitfield loads/stores [PR114313]
On Wed, 13 Mar 2024, Jakub Jelinek wrote:
> Hi!
>
> The following testcase ICEs, because for large/huge _BitInt bitfield
> loads/stores we use the DECL_BIT_FIELD_REPRESENTATIVE as the underlying
> "var" and indexes into it can be larger than the precision of the
> bitfield might normally allow.
>
> The following patch fixes that by passing NULL_TREE type in that case
> to limb_access, so that we always return m_limb_type type and don't
> do the extra assertions, after all, the callers expect that too.
> I had to add the first hunk to avoid ICE, it was using type in one place
> even when it was NULL. But TYPE_SIZE (TREE_TYPE (var)) seems like the
> right size to use anyway because the code uses VIEW_CONVERT_EXPR on it.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2024-03-13 Jakub Jelinek
>
> PR middle-end/114313
> * gimple-lower-bitint.cc (bitint_large_huge::limb_access): Use
> TYPE_SIZE of TREE_TYPE (var) rather than TYPE_SIZE of type.
> (bitint_large_huge::handle_load): Pass NULL_TREE rather than
> rhs_type to limb_access for the bitfield load cases.
> (bitint_large_huge::lower_mergeable_stmt): Pass NULL_TREE rather than
> lhs_type to limb_access if nlhs is non-NULL.
>
> * gcc.dg/torture/bitint-62.c: New test.
>
> --- gcc/gimple-lower-bitint.cc.jj 2024-03-05 10:27:04.609415622 +0100
> +++ gcc/gimple-lower-bitint.cc2024-03-12 16:45:50.152914901 +0100
> @@ -640,7 +640,7 @@ bitint_large_huge::limb_access (tree typ
>TREE_TYPE (TREE_TYPE (var
> {
> unsigned HOST_WIDE_INT nelts
> - = CEIL (tree_to_uhwi (TYPE_SIZE (type)), limb_prec);
> + = CEIL (tree_to_uhwi (TYPE_SIZE (TREE_TYPE (var))), limb_prec);
> tree atype = build_array_type_nelts (ltype, nelts);
> var = build1 (VIEW_CONVERT_EXPR, atype, var);
> }
> @@ -1854,7 +1854,7 @@ bitint_large_huge::handle_load (gimple *
> m_gsi = gsi_after_labels (gsi_bb (m_gsi));
> else
> gsi_next (_gsi);
> - tree t = limb_access (rhs_type, nrhs1, size_int (bo_idx), true);
> + tree t = limb_access (NULL_TREE, nrhs1, size_int (bo_idx), true);
> tree iv = make_ssa_name (m_limb_type);
> g = gimple_build_assign (iv, t);
> insert_before (g);
> @@ -1941,7 +1941,7 @@ bitint_large_huge::handle_load (gimple *
>tree iv2 = NULL_TREE;
>if (nidx0)
> {
> - tree t = limb_access (rhs_type, nrhs1, nidx0, true);
> + tree t = limb_access (NULL_TREE, nrhs1, nidx0, true);
> iv = make_ssa_name (m_limb_type);
> g = gimple_build_assign (iv, t);
> insert_before (g);
> @@ -1966,7 +1966,7 @@ bitint_large_huge::handle_load (gimple *
> if_then (g, profile_probability::likely (),
> edge_true, edge_false);
> }
> - tree t = limb_access (rhs_type, nrhs1, nidx1, true);
> + tree t = limb_access (NULL_TREE, nrhs1, nidx1, true);
> if (m_upwards_2limb
> && !m_first
> && !m_bitfld_load
> @@ -2728,8 +2728,8 @@ bitint_large_huge::lower_mergeable_stmt
> /* Otherwise, stores to any other lhs. */
> if (!done)
> {
> - tree l = limb_access (lhs_type, nlhs ? nlhs : lhs,
> - nidx, true);
> + tree l = limb_access (nlhs ? NULL_TREE : lhs_type,
> + nlhs ? nlhs : lhs, nidx, true);
> g = gimple_build_assign (l, rhs1);
> }
> insert_before (g);
> @@ -2873,7 +2873,8 @@ bitint_large_huge::lower_mergeable_stmt
> /* Otherwise, stores to any other lhs. */
> if (!done)
> {
> - tree l = limb_access (lhs_type, nlhs ? nlhs : lhs, nidx, true);
> + tree l = limb_access (nlhs ? NULL_TREE : lhs_type,
> + nlhs ? nlhs : lhs, nidx, true);
> g = gimple_build_assign (l, rhs1);
> }
> insert_before (g);
> --- gcc/testsuite/gcc.dg/torture/bitint-62.c.jj 2024-03-12
> 16:40:38.400198787 +0100
> +++ gcc/testsuite/gcc.dg/torture/bitint-62.c 2024-03-12 16:41:43.988297525
> +0100
> @@ -0,0 +1,28 @@
> +/* PR middle-end/114313 */
> +/* { dg-do run { target bitint } } */
> +/* { dg-options "-std=c23" } */
> +/* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O2" } } */
> +/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
> +
> +#if __BITINT_MAXWIDTH__ >= 256
> +struct S { _BitInt(257) : 257; _BitInt(256) b : 182; } s;
> +
> +__attribute__((noipa)) _BitInt(256)
> +foo (void)
> +{
> + return s.b;
> +}
> +#endif
> +
> +int
> +main ()
> +{
> +#if __BITINT_MAXWIDTH__ >= 256
> + s.b = 1414262180967678524960294186228886540125217087586381431wb;
> + if (foo () !=
[PATCH] bitint: Fix up lowering of bitfield loads/stores [PR114313]
Hi!
The following testcase ICEs, because for large/huge _BitInt bitfield
loads/stores we use the DECL_BIT_FIELD_REPRESENTATIVE as the underlying
"var" and indexes into it can be larger than the precision of the
bitfield might normally allow.
The following patch fixes that by passing NULL_TREE type in that case
to limb_access, so that we always return m_limb_type type and don't
do the extra assertions, after all, the callers expect that too.
I had to add the first hunk to avoid ICE, it was using type in one place
even when it was NULL. But TYPE_SIZE (TREE_TYPE (var)) seems like the
right size to use anyway because the code uses VIEW_CONVERT_EXPR on it.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2024-03-13 Jakub Jelinek
PR middle-end/114313
* gimple-lower-bitint.cc (bitint_large_huge::limb_access): Use
TYPE_SIZE of TREE_TYPE (var) rather than TYPE_SIZE of type.
(bitint_large_huge::handle_load): Pass NULL_TREE rather than
rhs_type to limb_access for the bitfield load cases.
(bitint_large_huge::lower_mergeable_stmt): Pass NULL_TREE rather than
lhs_type to limb_access if nlhs is non-NULL.
* gcc.dg/torture/bitint-62.c: New test.
--- gcc/gimple-lower-bitint.cc.jj 2024-03-05 10:27:04.609415622 +0100
+++ gcc/gimple-lower-bitint.cc 2024-03-12 16:45:50.152914901 +0100
@@ -640,7 +640,7 @@ bitint_large_huge::limb_access (tree typ
TREE_TYPE (TREE_TYPE (var
{
unsigned HOST_WIDE_INT nelts
- = CEIL (tree_to_uhwi (TYPE_SIZE (type)), limb_prec);
+ = CEIL (tree_to_uhwi (TYPE_SIZE (TREE_TYPE (var))), limb_prec);
tree atype = build_array_type_nelts (ltype, nelts);
var = build1 (VIEW_CONVERT_EXPR, atype, var);
}
@@ -1854,7 +1854,7 @@ bitint_large_huge::handle_load (gimple *
m_gsi = gsi_after_labels (gsi_bb (m_gsi));
else
gsi_next (_gsi);
- tree t = limb_access (rhs_type, nrhs1, size_int (bo_idx), true);
+ tree t = limb_access (NULL_TREE, nrhs1, size_int (bo_idx), true);
tree iv = make_ssa_name (m_limb_type);
g = gimple_build_assign (iv, t);
insert_before (g);
@@ -1941,7 +1941,7 @@ bitint_large_huge::handle_load (gimple *
tree iv2 = NULL_TREE;
if (nidx0)
{
- tree t = limb_access (rhs_type, nrhs1, nidx0, true);
+ tree t = limb_access (NULL_TREE, nrhs1, nidx0, true);
iv = make_ssa_name (m_limb_type);
g = gimple_build_assign (iv, t);
insert_before (g);
@@ -1966,7 +1966,7 @@ bitint_large_huge::handle_load (gimple *
if_then (g, profile_probability::likely (),
edge_true, edge_false);
}
- tree t = limb_access (rhs_type, nrhs1, nidx1, true);
+ tree t = limb_access (NULL_TREE, nrhs1, nidx1, true);
if (m_upwards_2limb
&& !m_first
&& !m_bitfld_load
@@ -2728,8 +2728,8 @@ bitint_large_huge::lower_mergeable_stmt
/* Otherwise, stores to any other lhs. */
if (!done)
{
- tree l = limb_access (lhs_type, nlhs ? nlhs : lhs,
- nidx, true);
+ tree l = limb_access (nlhs ? NULL_TREE : lhs_type,
+ nlhs ? nlhs : lhs, nidx, true);
g = gimple_build_assign (l, rhs1);
}
insert_before (g);
@@ -2873,7 +2873,8 @@ bitint_large_huge::lower_mergeable_stmt
/* Otherwise, stores to any other lhs. */
if (!done)
{
- tree l = limb_access (lhs_type, nlhs ? nlhs : lhs, nidx, true);
+ tree l = limb_access (nlhs ? NULL_TREE : lhs_type,
+ nlhs ? nlhs : lhs, nidx, true);
g = gimple_build_assign (l, rhs1);
}
insert_before (g);
--- gcc/testsuite/gcc.dg/torture/bitint-62.c.jj 2024-03-12 16:40:38.400198787
+0100
+++ gcc/testsuite/gcc.dg/torture/bitint-62.c2024-03-12 16:41:43.988297525
+0100
@@ -0,0 +1,28 @@
+/* PR middle-end/114313 */
+/* { dg-do run { target bitint } } */
+/* { dg-options "-std=c23" } */
+/* { dg-skip-if "" { ! run_expensive_tests } { "*" } { "-O0" "-O2" } } */
+/* { dg-skip-if "" { ! run_expensive_tests } { "-flto" } { "" } } */
+
+#if __BITINT_MAXWIDTH__ >= 256
+struct S { _BitInt(257) : 257; _BitInt(256) b : 182; } s;
+
+__attribute__((noipa)) _BitInt(256)
+foo (void)
+{
+ return s.b;
+}
+#endif
+
+int
+main ()
+{
+#if __BITINT_MAXWIDTH__ >= 256
+ s.b = 1414262180967678524960294186228886540125217087586381431wb;
+ if (foo () != 1414262180967678524960294186228886540125217087586381431wb)
+__builtin_abort ();
+ s.b = -581849792837428541666755934071828568425158644418477999wb;
+ if (foo () !=
