Re: [PATCH] bitint: Fix up stores to large/huge _BitInt bitfields [PR114329]

[Richard Biener <rguenther@suse.de>]

> Am 16.03.2024 um 08:11 schrieb Jakub Jelinek <jakub@redhat.com>:
> 
> Hi!
> 
> The verifier requires BIT_FIELD_REFs with INTEGRAL_TYPE_P first operand
> to have mode precision.  In most cases for the large/huge _BitInt bitfield
> stores the code uses bitfield representatives, which are typically arrays
> of chars, but if the bitfield starts at byte boundary on big endian,
> the code uses as nlhs in lower_mergeable_store COMPONENT_REF of the
> bitfield FIELD_DECL instead, which is fine for the limb accesses,
> but when used for the most significant limb can result in invalid
> BIT_FIELD_REF because the first operand then has BITINT_TYPE and
> usually VOIDmode.
> 
> The following patch adds a helper method for the 4 creatikons of
> BIT_FIELD_REF which when needed adds a VIEW_CONVERT_EXPR.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Ok

Richard 

> 2024-03-16  Jakub Jelinek  <jakub@redhat.com>
> 
>    PR tree-optimization/114329
>    * gimple-lower-bitint.cc (struct bitint_large_huge): Declare
>    build_bit_field_ref method.
>    (bitint_large_huge::build_bit_field_ref): New method.
>    (bitint_large_huge::lower_mergeable_stmt): Use it.
> 
>    * gcc.dg/bitint-101.c: New test.
> 
> --- gcc/gimple-lower-bitint.cc.jj    2024-03-15 09:16:36.464033118 +0100
> +++ gcc/gimple-lower-bitint.cc    2024-03-15 17:53:30.200276578 +0100
> @@ -427,6 +427,8 @@ struct bitint_large_huge
>   void insert_before (gimple *);
>   tree limb_access_type (tree, tree);
>   tree limb_access (tree, tree, tree, bool);
> +  tree build_bit_field_ref (tree, tree, unsigned HOST_WIDE_INT,
> +                unsigned HOST_WIDE_INT);
>   void if_then (gimple *, profile_probability, edge &, edge &);
>   void if_then_else (gimple *, profile_probability, edge &, edge &);
>   void if_then_if_then_else (gimple *g, gimple *,
> @@ -659,6 +661,31 @@ bitint_large_huge::limb_access (tree typ
>   return ret;
> }
> 
> +/* Build a BIT_FIELD_REF to access BITSIZE bits with FTYPE type at
> +   offset BITPOS inside of OBJ.  */
> +
> +tree
> +bitint_large_huge::build_bit_field_ref (tree ftype, tree obj,
> +                    unsigned HOST_WIDE_INT bitsize,
> +                    unsigned HOST_WIDE_INT bitpos)
> +{
> +  if (INTEGRAL_TYPE_P (TREE_TYPE (obj))
> +      && !type_has_mode_precision_p (TREE_TYPE (obj)))
> +    {
> +      unsigned HOST_WIDE_INT nelts
> +    = CEIL (tree_to_uhwi (TYPE_SIZE (TREE_TYPE (obj))), limb_prec);
> +      tree ltype = m_limb_type;
> +      addr_space_t as = TYPE_ADDR_SPACE (TREE_TYPE (obj));
> +      if (as != TYPE_ADDR_SPACE (ltype))
> +    ltype = build_qualified_type (ltype, TYPE_QUALS (ltype)
> +                      | ENCODE_QUAL_ADDR_SPACE (as));
> +      tree atype = build_array_type_nelts (ltype, nelts);
> +      obj = build1 (VIEW_CONVERT_EXPR, atype, obj);
> +    }
> +  return build3 (BIT_FIELD_REF, ftype, obj, bitsize_int (bitsize),
> +         bitsize_int (bitpos));
> +}
> +
> /* Emit a half diamond,
>    if (COND)
>      |\
> @@ -2651,9 +2678,9 @@ bitint_large_huge::lower_mergeable_stmt
>            }
>          tree ftype
>            = build_nonstandard_integer_type (limb_prec - bo_bit, 1);
> -          tree bfr = build3 (BIT_FIELD_REF, ftype, unshare_expr (nlhs),
> -                     bitsize_int (limb_prec - bo_bit),
> -                     bitsize_int (bo_idx * limb_prec + bo_bit));
> +          tree bfr = build_bit_field_ref (ftype, unshare_expr (nlhs),
> +                          limb_prec - bo_bit,
> +                          bo_idx * limb_prec + bo_bit);
>          tree t = add_cast (ftype, rhs1);
>          g = gimple_build_assign (bfr, t);
>          insert_before (g);
> @@ -2714,12 +2741,11 @@ bitint_large_huge::lower_mergeable_stmt
>            {
>              tree ftype
>            = build_nonstandard_integer_type (rprec + bo_bit, 1);
> -              tree bfr = build3 (BIT_FIELD_REF, ftype,
> -                     unshare_expr (nlhs),
> -                     bitsize_int (rprec + bo_bit),
> -                     bitsize_int ((bo_idx
> -                               + tprec / limb_prec)
> -                              * limb_prec));
> +              tree bfr
> +            = build_bit_field_ref (ftype, unshare_expr (nlhs),
> +                           rprec + bo_bit,
> +                           (bo_idx + tprec / limb_prec)
> +                           * limb_prec);
>              tree t = add_cast (ftype, rhs1);
>              g = gimple_build_assign (bfr, t);
>              done = true;
> @@ -2860,11 +2886,11 @@ bitint_large_huge::lower_mergeable_stmt
>        {
>          tree ftype
>            = build_nonstandard_integer_type (rprec + bo_bit, 1);
> -          tree bfr = build3 (BIT_FIELD_REF, ftype,
> -                     unshare_expr (nlhs),
> -                     bitsize_int (rprec + bo_bit),
> -                     bitsize_int ((bo_idx + tprec / limb_prec)
> -                          * limb_prec));
> +          tree bfr
> +            = build_bit_field_ref (ftype, unshare_expr (nlhs),
> +                       rprec + bo_bit,
> +                       (bo_idx + tprec / limb_prec)
> +                       * limb_prec);
>          tree t = add_cast (ftype, rhs1);
>          g = gimple_build_assign (bfr, t);
>          done = true;
> @@ -2909,10 +2935,10 @@ bitint_large_huge::lower_mergeable_stmt
>       unsigned int tprec = TYPE_PRECISION (type);
>       unsigned int rprec = tprec % limb_prec;
>       tree ftype = build_nonstandard_integer_type (rprec + bo_bit, 1);
> -      tree bfr = build3 (BIT_FIELD_REF, ftype, unshare_expr (nlhs),
> -             bitsize_int (rprec + bo_bit),
> -             bitsize_int ((bo_idx + tprec / limb_prec)
> -                      * limb_prec));
> +      tree bfr = build_bit_field_ref (ftype, unshare_expr (nlhs),
> +                      rprec + bo_bit,
> +                      (bo_idx + tprec / limb_prec)
> +                      * limb_prec);
>       rhs1 = bf_cur;
>       if (bf_cur != ext)
>    {
> --- gcc/testsuite/gcc.dg/bitint-101.c.jj    2024-03-15 18:59:01.386413294 +0100
> +++ gcc/testsuite/gcc.dg/bitint-101.c    2024-03-15 18:58:14.063059994 +0100
> @@ -0,0 +1,17 @@
> +/* PR tree-optimization/114329 */
> +/* { dg-do compile { target bitint } } */
> +/* { dg-options "-std=c23" } */
> +
> +#if __BITINT_MAXWIDTH__ >= 129
> +#define N 129
> +#else
> +#define N 63
> +#endif
> +
> +struct S { _BitInt(N) b : N; } s;
> +
> +void
> +foo (void)
> +{
> +  s.b ^= 42;
> +}
> 
>    Jakub
>