Re: [PATCH v9 2/5] Convert references with "counted_by" attributes to/from .ACCESS_WITH_SIZE.

[Qing Zhao <qing.zhao@oracle.com>]

> On May 29, 2024, at 02:57, Richard Biener <richard.guenther@gmail.com> wrote:
> 
> On Tue, May 28, 2024 at 11:09 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>> 
>> Thank you for the comments. See my answers below:
>> 
>> Joseph, please see the last question, I need your help on it. Thanks a lot for the help.
>> 
>> Qing
>> 
>>> On May 28, 2024, at 03:38, Richard Biener <richard.guenther@gmail.com> wrote:
>>> 
>>> On Fri, Apr 12, 2024 at 3:54 PM Qing Zhao <qing.zhao@oracle.com> wrote:
>>>> 
>>>> Including the following changes:
>>>> * The definition of the new internal function .ACCESS_WITH_SIZE
>>>> in internal-fn.def.
>>>> * C FE converts every reference to a FAM with a "counted_by" attribute
>>>> to a call to the internal function .ACCESS_WITH_SIZE.
>>>> (build_component_ref in c_typeck.cc)
>>>> 
>>>> This includes the case when the object is statically allocated and
>>>> initialized.
>>>> In order to make this working, the routines initializer_constant_valid_p_1
>>>> and output_constant in varasm.cc are updated to handle calls to
>>>> .ACCESS_WITH_SIZE.
>>>> (initializer_constant_valid_p_1 and output_constant in varasm.c)
>>>> 
>>>> However, for the reference inside "offsetof", the "counted_by" attribute is
>>>> ignored since it's not useful at all.
>>>> (c_parser_postfix_expression in c/c-parser.cc)
>>>> 
>>>> In addtion to "offsetof", for the reference inside operator "typeof" and
>>>> "alignof", we ignore counted_by attribute too.
>>>> 
>>>> When building ADDR_EXPR for the .ACCESS_WITH_SIZE in C FE,
>>>> replace the call with its first argument.
>>>> 
>>>> * Convert every call to .ACCESS_WITH_SIZE to its first argument.
>>>> (expand_ACCESS_WITH_SIZE in internal-fn.cc)
>>>> * Adjust alias analysis to exclude the new internal from clobbering anything.
>>>> (ref_maybe_used_by_call_p_1 and call_may_clobber_ref_p_1 in tree-ssa-alias.cc)
>>>> * Adjust dead code elimination to eliminate the call to .ACCESS_WITH_SIZE when
>>>> it's LHS is eliminated as dead code.
>>>> (eliminate_unnecessary_stmts in tree-ssa-dce.cc)
>>>> * Provide the utility routines to check the call is .ACCESS_WITH_SIZE and
>>>> get the reference from the call to .ACCESS_WITH_SIZE.
>>>> (is_access_with_size_p and get_ref_from_access_with_size in tree.cc)
>>>> 
>>>> gcc/c/ChangeLog:
>>>> 
>>>>       * c-parser.cc (c_parser_postfix_expression): Ignore the counted-by
>>>>       attribute when build_component_ref inside offsetof operator.
>>>>       * c-tree.h (build_component_ref): Add one more parameter.
>>>>       * c-typeck.cc (build_counted_by_ref): New function.
>>>>       (build_access_with_size_for_counted_by): New function.
>>>>       (build_component_ref): Check the counted-by attribute and build
>>>>       call to .ACCESS_WITH_SIZE.
>>>>       (build_unary_op): When building ADDR_EXPR for
>>>>       .ACCESS_WITH_SIZE, use its first argument.
>>>>       (lvalue_p): Accept call to .ACCESS_WITH_SIZE.
>>>> 
>>>> gcc/ChangeLog:
>>>> 
>>>>       * internal-fn.cc (expand_ACCESS_WITH_SIZE): New function.
>>>>       * internal-fn.def (ACCESS_WITH_SIZE): New internal function.
>>>>       * tree-ssa-alias.cc (ref_maybe_used_by_call_p_1): Special case
>>>>       IFN_ACCESS_WITH_SIZE.
>>>>       (call_may_clobber_ref_p_1): Special case IFN_ACCESS_WITH_SIZE.
>>>>       * tree-ssa-dce.cc (eliminate_unnecessary_stmts): Eliminate the call
>>>>       to .ACCESS_WITH_SIZE when its LHS is dead.
>>>>       * tree.cc (process_call_operands): Adjust side effect for function
>>>>       .ACCESS_WITH_SIZE.
>>>>       (is_access_with_size_p): New function.
>>>>       (get_ref_from_access_with_size): New function.
>>>>       * tree.h (is_access_with_size_p): New prototype.
>>>>       (get_ref_from_access_with_size): New prototype.
>>>>       * varasm.cc (initializer_constant_valid_p_1): Handle call to
>>>>       .ACCESS_WITH_SIZE.
>>>>       (output_constant): Handle call to .ACCESS_WITH_SIZE.
>>>> 
>>>> gcc/testsuite/ChangeLog:
>>>> 
>>>>       * gcc.dg/flex-array-counted-by-2.c: New test.
>>>> ---
>>>> gcc/c/c-parser.cc                             |  10 +-
>>>> gcc/c/c-tree.h                                |   2 +-
>>>> gcc/c/c-typeck.cc                             | 128 +++++++++++++++++-
>>>> gcc/internal-fn.cc                            |  35 +++++
>>>> gcc/internal-fn.def                           |   4 +
>>>> .../gcc.dg/flex-array-counted-by-2.c          | 112 +++++++++++++++
>>>> gcc/tree-ssa-alias.cc                         |   2 +
>>>> gcc/tree-ssa-dce.cc                           |   5 +-
>>>> gcc/tree.cc                                   |  25 +++-
>>>> gcc/tree.h                                    |   8 ++
>>>> gcc/varasm.cc                                 |  10 ++
>>>> 11 files changed, 331 insertions(+), 10 deletions(-)
>>>> create mode 100644 gcc/testsuite/gcc.dg/flex-array-counted-by-2.c
>>>> 
>>>> diff --git a/gcc/c/c-parser.cc b/gcc/c/c-parser.cc
>>>> index c31349dae2ff..a6ed5ac43bb1 100644
>>>> --- a/gcc/c/c-parser.cc
>>>> +++ b/gcc/c/c-parser.cc
>>>> @@ -10850,9 +10850,12 @@ c_parser_postfix_expression (c_parser *parser)
>>>>           if (c_parser_next_token_is (parser, CPP_NAME))
>>>>             {
>>>>               c_token *comp_tok = c_parser_peek_token (parser);
>>>> +               /* Ignore the counted_by attribute for reference inside
>>>> +                  offsetof since the information is not useful at all.  */
>>>>               offsetof_ref
>>>>                 = build_component_ref (loc, offsetof_ref, comp_tok->value,
>>>> -                                        comp_tok->location, UNKNOWN_LOCATION);
>>>> +                                        comp_tok->location, UNKNOWN_LOCATION,
>>>> +                                        false);
>>>>               c_parser_consume_token (parser);
>>>>               while (c_parser_next_token_is (parser, CPP_DOT)
>>>>                      || c_parser_next_token_is (parser,
>>>> @@ -10879,11 +10882,14 @@ c_parser_postfix_expression (c_parser *parser)
>>>>                           break;
>>>>                         }
>>>>                       c_token *comp_tok = c_parser_peek_token (parser);
>>>> +                       /* Ignore the counted_by attribute for reference inside
>>>> +                          offsetof since the information is not useful.  */
>>>>                       offsetof_ref
>>>>                         = build_component_ref (loc, offsetof_ref,
>>>>                                                comp_tok->value,
>>>>                                                comp_tok->location,
>>>> -                                                UNKNOWN_LOCATION);
>>>> +                                                UNKNOWN_LOCATION,
>>>> +                                                false);
>>>>                       c_parser_consume_token (parser);
>>>>                     }
>>>>                   else
>>>> diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
>>>> index 12fae8591462..402e8f78db2a 100644
>>>> --- a/gcc/c/c-tree.h
>>>> +++ b/gcc/c/c-tree.h
>>>> @@ -778,7 +778,7 @@ extern void mark_exp_read (tree);
>>>> extern tree composite_type (tree, tree);
>>>> extern tree lookup_field (const_tree, tree);
>>>> extern tree build_component_ref (location_t, tree, tree, location_t,
>>>> -                                location_t);
>>>> +                                location_t, bool = true);
>>>> extern tree build_array_ref (location_t, tree, tree);
>>>> extern tree build_omp_array_section (location_t, tree, tree, tree);
>>>> extern tree build_external_ref (location_t, tree, bool, tree *);
>>>> diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
>>>> index fb7587f05f1f..ff6685c6c4ba 100644
>>>> --- a/gcc/c/c-typeck.cc
>>>> +++ b/gcc/c/c-typeck.cc
>>>> @@ -2578,15 +2578,116 @@ should_suggest_deref_p (tree datum_type)
>>>>    return false;
>>>> }
>>>> 
>>>> +/* For a SUBDATUM field of a structure or union DATUM, generate a REF to
>>>> +   the object that represents its counted_by per the attribute counted_by
>>>> +   attached to this field if it's a flexible array member field, otherwise
>>>> +   return NULL_TREE.
>>>> +   Set COUNTED_BY_TYPE to the TYPE of the counted_by field.
>>>> +   For example, if:
>>>> +
>>>> +    struct P {
>>>> +      int k;
>>>> +      int x[] __attribute__ ((counted_by (k)));
>>>> +    } *p;
>>>> +
>>>> +    for:
>>>> +    p->x
>>>> +
>>>> +    the ref to the object that represents its element count will be:
>>>> +
>>>> +    &(p->k)
>>>> +
>>>> +*/
>>>> +static tree
>>>> +build_counted_by_ref (tree datum, tree subdatum, tree *counted_by_type)
>>>> +{
>>>> +  tree type = TREE_TYPE (datum);
>>>> +  if (!c_flexible_array_member_type_p (TREE_TYPE (subdatum)))
>>>> +    return NULL_TREE;
>>>> +
>>>> +  tree attr_counted_by = lookup_attribute ("counted_by",
>>>> +                                          DECL_ATTRIBUTES (subdatum));
>>>> +  tree counted_by_ref = NULL_TREE;
>>>> +  *counted_by_type = NULL_TREE;
>>>> +  if (attr_counted_by)
>>>> +    {
>>>> +      tree field_id = TREE_VALUE (TREE_VALUE (attr_counted_by));
>>>> +      counted_by_ref
>>>> +       = build_component_ref (UNKNOWN_LOCATION,
>>>> +                              datum, field_id,
>>>> +                              UNKNOWN_LOCATION, UNKNOWN_LOCATION);
>>>> +      counted_by_ref = build_fold_addr_expr (counted_by_ref);
>>>> +
>>>> +      /* Get the TYPE of the counted_by field.  */
>>>> +      tree counted_by_field = lookup_field (type, field_id);
>>>> +      gcc_assert (counted_by_field);
>>>> +
>>>> +      do
>>>> +       {
>>>> +         *counted_by_type = TREE_TYPE (TREE_VALUE (counted_by_field));
>>>> +         counted_by_field = TREE_CHAIN (counted_by_field);
>>>> +       }
>>>> +      while (counted_by_field);
>>>> +    }
>>>> +  return counted_by_ref;
>>>> +}
>>>> +
>>>> +/* Given a COMPONENT_REF REF with the location LOC, the corresponding
>>>> +   COUNTED_BY_REF, and the COUNTED_BY_TYPE, generate an INDIRECT_REF
>>>> +   to a call to the internal function .ACCESS_WITH_SIZE.
>>>> +
>>>> +   REF
>>>> +
>>>> +   to:
>>>> +
>>>> +   (*.ACCESS_WITH_SIZE (REF, COUNTED_BY_REF, 1, (TYPE_OF_SIZE)0, -1))
>>>> +
>>>> +   NOTE: The return type of this function is the POINTER type pointing
>>>> +   to the original flexible array type.
>>>> +   Then the type of the INDIRECT_REF is the original flexible array type.
>>>> +
>>>> +   The type of the first argument of this function is a POINTER type
>>>> +   to the original flexible array type.
>>>> +
>>>> +   The 4th argument of the call is a constant 0 with the TYPE of the
>>>> +   object pointed by COUNTED_BY_REF.
>>>> +
>>>> +  */
>>>> +static tree
>>>> +build_access_with_size_for_counted_by (location_t loc, tree ref,
>>>> +                                      tree counted_by_ref,
>>>> +                                      tree counted_by_type)
>>>> +{
>>>> +  gcc_assert (c_flexible_array_member_type_p (TREE_TYPE (ref)));
>>>> +  /* The result type of the call is a pointer to the flexible array type.  */
>>>> +  tree result_type = build_pointer_type (TREE_TYPE (ref));
>>>> +
>>>> +  tree call
>>>> +    = build_call_expr_internal_loc (loc, IFN_ACCESS_WITH_SIZE,
>>>> +                                   result_type, 5,
>>>> +                                   array_to_pointer_conversion (loc, ref),
>>>> +                                   counted_by_ref,
>>>> +                                   build_int_cst (integer_type_node, 1),
>>>> +                                   build_int_cst (counted_by_type, 0),
>>>> +                                   build_int_cst (integer_type_node, -1));
>>>> +  /* Wrap the call with an INDIRECT_REF with the flexible array type.  */
>>>> +  call = build1 (INDIRECT_REF, TREE_TYPE (ref), call);
>>>> +  SET_EXPR_LOCATION (call, loc);
>>>> +  return call;
>>>> +}
>>>> +
>>>> /* Make an expression to refer to the COMPONENT field of structure or
>>>>   union value DATUM.  COMPONENT is an IDENTIFIER_NODE.  LOC is the
>>>>   location of the COMPONENT_REF.  COMPONENT_LOC is the location
>>>>   of COMPONENT.  ARROW_LOC is the location of the first -> operand if
>>>> -   it is from -> operator.  */
>>>> +   it is from -> operator.
>>>> +   If HANDLE_COUNTED_BY is true, check the counted_by attribute and generate
>>>> +   a call to .ACCESS_WITH_SIZE.  Otherwise, ignore the attribute.  */
>>>> 
>>>> tree
>>>> build_component_ref (location_t loc, tree datum, tree component,
>>>> -                    location_t component_loc, location_t arrow_loc)
>>>> +                    location_t component_loc, location_t arrow_loc,
>>>> +                    bool handle_counted_by)
>>>> {
>>>>  tree type = TREE_TYPE (datum);
>>>>  enum tree_code code = TREE_CODE (type);
>>>> @@ -2658,7 +2759,13 @@ build_component_ref (location_t loc, tree datum, tree component,
>>>>         int quals;
>>>>         tree subtype;
>>>>         bool use_datum_quals;
>>>> -
>>>> +         tree counted_by_type = NULL_TREE;
>>>> +         /* Do not handle counted_by when in typeof and alignof operator.  */
>>>> +         handle_counted_by = handle_counted_by && !in_typeof && !in_alignof;
>>>> +         tree counted_by_ref = handle_counted_by
>>>> +                               ? build_counted_by_ref (datum, subdatum,
>>>> +                                                       &counted_by_type)
>>>> +                               : NULL_TREE;
>>>>         if (TREE_TYPE (subdatum) == error_mark_node)
>>>>           return error_mark_node;
>>>> 
>>>> @@ -2677,6 +2784,12 @@ build_component_ref (location_t loc, tree datum, tree component,
>>>>         ref = build3 (COMPONENT_REF, subtype, datum, subdatum,
>>>>                       NULL_TREE);
>>>>         SET_EXPR_LOCATION (ref, loc);
>>>> +
>>>> +         if (counted_by_ref)
>>>> +           ref = build_access_with_size_for_counted_by (loc, ref,
>>>> +                                                        counted_by_ref,
>>>> +                                                        counted_by_type);
>>>> +
>>>>         if (TREE_READONLY (subdatum)
>>>>             || (use_datum_quals && TREE_READONLY (datum)))
>>>>           TREE_READONLY (ref) = 1;
>>>> @@ -5080,7 +5193,11 @@ build_unary_op (location_t location, enum tree_code code, tree xarg,
>>>>         goto return_build_unary_op;
>>>>       }
>>>> 
>>>> -      /* Ordinary case; arg is a COMPONENT_REF or a decl.  */
>>>> +      /* Ordinary case; arg is a COMPONENT_REF or a decl, or a call to
>>>> +        .ACCESS_WITH_SIZE.  */
>>>> +      if (is_access_with_size_p (arg))
>>>> +       arg = TREE_OPERAND (TREE_OPERAND (CALL_EXPR_ARG (arg, 0), 0), 0);
>>>> +
>>>>      argtype = TREE_TYPE (arg);
>>>> 
>>>>      /* If the lvalue is const or volatile, merge that into the type
>>>> @@ -5227,6 +5344,9 @@ lvalue_p (const_tree ref)
>>>>    case BIND_EXPR:
>>>>      return TREE_CODE (TREE_TYPE (ref)) == ARRAY_TYPE;
>>>> 
>>>> +    case CALL_EXPR:
>>>> +      return is_access_with_size_p (ref);
>>>> +
>>>>    default:
>>>>      return false;
>>>>    }
>>>> diff --git a/gcc/internal-fn.cc b/gcc/internal-fn.cc
>>>> index a07f25f3aee3..e744080ee670 100644
>>>> --- a/gcc/internal-fn.cc
>>>> +++ b/gcc/internal-fn.cc
>>>> @@ -3393,6 +3393,41 @@ expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>>>>    }
>>>> }
>>>> 
>>>> +/* Expand the IFN_ACCESS_WITH_SIZE function:
>>>> +   ACCESS_WITH_SIZE (REF_TO_OBJ, REF_TO_SIZE, CLASS_OF_SIZE,
>>>> +                    TYPE_OF_SIZE, ACCESS_MODE)
>>>> +   which returns the REF_TO_OBJ same as the 1st argument;
>>>> +
>>>> +   1st argument REF_TO_OBJ: The reference to the object;
>>>> +   2nd argument REF_TO_SIZE: The reference to the size of the object,
>>>> +   3rd argument CLASS_OF_SIZE: The size referenced by the REF_TO_SIZE represents
>>>> +     0: the number of bytes.
>>>> +     1: the number of the elements of the object type;
>>>> +   4th argument TYPE_OF_SIZE: A constant 0 with its TYPE being the same as the TYPE
>>>> +    of the object referenced by REF_TO_SIZE
>>>> +   5th argument ACCESS_MODE:
>>>> +    -1: Unknown access semantics
>>>> +     0: none
>>>> +     1: read_only
>>>> +     2: write_only
>>>> +     3: read_write
>>>> +
>>>> +   Both the return type and the type of the first argument of this
>>>> +   function have been converted from the incomplete array type to
>>>> +   the corresponding pointer type.
>>>> +
>>>> +   For each call to a .ACCESS_WITH_SIZE, replace it with its 1st argument.  */
>>>> +static void
>>>> +expand_ACCESS_WITH_SIZE (internal_fn, gcall *stmt)
>>>> +{
>>>> +  tree lhs = gimple_call_lhs (stmt);
>>>> +  tree ref_to_obj = gimple_call_arg (stmt, 0);
>>>> +  if (lhs)
>>>> +    expand_assignment (lhs, ref_to_obj, false);
>>>> +  else
>>>> +    emit_insn (expand_normal (ref_to_obj));
>>> 
>>> That looks suspicious, expand_normal does not result in an insn.  What is
>>> there to do when there's no LHS of the .ACCESS_WITH_SIZE?
>> 
>> I think my initial purpose of that line is trying to handle such rare situation that might happen with different optimizations..
>> I can change the else path to a gcc_unreachable() to see any issue.
>> What do you think?
> 
> As I've seen later in DCE you happily elide .ACCESS_WITH_SIZE without
> a LHS so I'd
> say you should simply do nothing (remove the else) here which means
> DCEing the call.

Okay, will do that. Thanks.
> 
>>> 
>>>> +}
>>>> +
>>>> /* The size of an OpenACC compute dimension.  */
>>>> 
>>>> static void
>>>> diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
>>>> index c14d30365c14..0801c8bfe61d 100644
>>>> --- a/gcc/internal-fn.def
>>>> +++ b/gcc/internal-fn.def
>>>> @@ -510,6 +510,10 @@ DEF_INTERNAL_FN (PHI, 0, NULL)
>>>>   automatic variable.  */
>>>> DEF_INTERNAL_FN (DEFERRED_INIT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
>>>> 
>>>> +/* A function to associate the access size and access mode information
>>>> +   with the corresponding reference to an object.  */
>>>> +DEF_INTERNAL_FN (ACCESS_WITH_SIZE, ECF_LEAF | ECF_NOTHROW, NULL)
>>> 
>>> Does .ACCESS_WITH_SIZE store to memory?  (or even load?)
>> It reads from the 2nd argument.
>> So, it’s not PURE.
> 
> If it only reads it _is_ PURE.  It's not CONST.

Just read the documentation of GCC pure attribute:
https://gcc.gnu.org/onlinedocs/gcc/Common-Function-Attributes.html#index-pure-function-attribute

So, ECF_PURE has the same meaning of the pure attribute? 
If so, yes, .ACCESS_WITH_SIZE is ECF_PURE. 

Thanks a lot for catching this bug.

Will update it. 
> 
>> Please see our discussion last November on this:
>> https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635158.html
>> 
>> In which: "Although .ACCESS_WITH_SIZE is not PURE anymore, but it’s only read from the 2nd argument, and not modify anything in the pointed objects. So, we can adjust the IPA alias analysis phase with this details (ref_maybe_used_by_call_p_1/call_may_clobber_ref_p_1).”
>> 
>> (NOTE, in the initial proposal (https://gcc.gnu.org/pipermail/gcc-patches/2023-October/634844.html) I defined it as
>> DEF_INTERNAL_FN (ACCESS_WITH_SIZE, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL),
>> And later after discussion, we deleted ECF_CONST from it)
> 
> And you now should put in ECF_PURE.
Okay.
> 
>> 
>>> 
>>>> +
>>>> /* DIM_SIZE and DIM_POS return the size of a particular compute
>>>>   dimension and the executing thread's position within that
>>>>   dimension.  DIM_POS is pure (and not const) so that it isn't
>>>> diff --git a/gcc/testsuite/gcc.dg/flex-array-counted-by-2.c b/gcc/testsuite/gcc.dg/flex-array-counted-by-2.c
>>>> new file mode 100644
>>>> index 000000000000..d4899a63af3c
>>>> --- /dev/null
>>>> +++ b/gcc/testsuite/gcc.dg/flex-array-counted-by-2.c
>>>> @@ -0,0 +1,112 @@
>>>> +/* Test the code generation for the new attribute counted_by.
>>>> +   And also the offsetof operator on such array.  */
>>>> +/* { dg-do run } */
>>>> +/* { dg-options "-O2 -fdump-tree-original" } */
>>>> +
>>>> +#include <stdlib.h>
>>>> +
>>>> +struct annotated {
>>>> +  int b;
>>>> +  char c[] __attribute__ ((counted_by (b)));
>>>> +} *array_annotated;
>>>> +
>>>> +static struct annotated static_annotated = { sizeof "hello", "hello" };
>>>> +static char *y = static_annotated.c;
>>>> +
>>>> +struct flex {
>>>> +  int b;
>>>> +  char c[];
>>>> +};
>>>> +
>>>> +struct nested_annotated {
>>>> +  struct {
>>>> +    union {
>>>> +      int b;
>>>> +      float f;
>>>> +    };
>>>> +    int n;
>>>> +  };
>>>> +  char c[] __attribute__ ((counted_by (b)));
>>>> +} *array_nested_annotated;
>>>> +
>>>> +static struct nested_annotated nested_static_annotated
>>>> +                                = { sizeof "hello1", 0, "hello1" };
>>>> +static char *nested_y = nested_static_annotated.c;
>>>> +
>>>> +struct nested_flex {
>>>> +  struct {
>>>> +    union {
>>>> +      int b;
>>>> +      float f;
>>>> +    };
>>>> +    int n;
>>>> +  };
>>>> +  char c[];
>>>> +};
>>>> +
>>>> +void __attribute__((__noinline__)) setup (int normal_count, int attr_count)
>>>> +{
>>>> +  array_annotated
>>>> +    = (struct annotated *)malloc (sizeof (struct annotated)
>>>> +                                 + attr_count *  sizeof (char));
>>>> +  array_annotated->b = attr_count;
>>>> +
>>>> +  array_nested_annotated
>>>> +    = (struct nested_annotated *)malloc (sizeof (struct nested_annotated)
>>>> +                                        + attr_count *  sizeof (char));
>>>> +  array_nested_annotated->b = attr_count;
>>>> +
>>>> +  return;
>>>> +}
>>>> +
>>>> +void __attribute__((__noinline__)) test (char a, char b)
>>>> +{
>>>> +  if (__builtin_offsetof (struct annotated, c[0])
>>>> +      != __builtin_offsetof (struct flex, c[0]))
>>>> +    abort ();
>>>> +  if (__builtin_offsetof (struct annotated, c[1])
>>>> +      != __builtin_offsetof (struct flex, c[1]))
>>>> +    abort ();
>>>> +  if (__builtin_offsetof (struct nested_annotated, c[0])
>>>> +      != __builtin_offsetof (struct nested_flex, c[0]))
>>>> +    abort ();
>>>> +  if (__builtin_offsetof (struct nested_annotated, c[1])
>>>> +      != __builtin_offsetof (struct nested_flex, c[1]))
>>>> +    abort ();
>>>> +
>>>> +  if (__builtin_types_compatible_p (typeof (array_annotated->c),
>>>> +                                   typeof (&(array_annotated->c)[0])))
>>>> +    abort ();
>>>> +  if (__builtin_types_compatible_p (typeof (array_nested_annotated->c),
>>>> +                                   typeof (&(array_nested_annotated->c)[0])))
>>>> +    abort ();
>>>> +
>>>> +  if (__alignof (array_annotated->c) != __alignof (char))
>>>> +    abort ();
>>>> +  if (__alignof (array_nested_annotated->c) != __alignof (char))
>>>> +    abort ();
>>>> +
>>>> +  if ((unsigned long) array_annotated->c != (unsigned long) &array_annotated->c)
>>>> +    abort ();
>>>> +  if ((unsigned long) array_nested_annotated->c
>>>> +       != (unsigned long) &array_nested_annotated->c)
>>>> +    abort ();
>>>> +
>>>> +  array_annotated->c[2] = a;
>>>> +  array_nested_annotated->c[3] = b;
>>>> +
>>>> +  if (y[2] != 'l') abort ();
>>>> +  if (nested_y[4] !='o') abort ();
>>>> +
>>>> +}
>>>> +
>>>> +int main(int argc, char *argv[])
>>>> +{
>>>> +  setup (10,10);
>>>> +  test ('A', 'B');
>>>> +  if (array_annotated->c[2] != 'A') abort ();
>>>> +  if (array_nested_annotated->c[3] != 'B') abort ();
>>>> +  return 0;
>>>> +}
>>>> +
>>>> +/* { dg-final { scan-tree-dump-times "ACCESS_WITH_SIZE" 8 "original" } } */
>>>> diff --git a/gcc/tree-ssa-alias.cc b/gcc/tree-ssa-alias.cc
>>>> index e7c1c1aa6243..8c070e173bdd 100644
>>>> --- a/gcc/tree-ssa-alias.cc
>>>> +++ b/gcc/tree-ssa-alias.cc
>>>> @@ -2823,6 +2823,7 @@ ref_maybe_used_by_call_p_1 (gcall *call, ao_ref *ref, bool tbaa_p)
>>>>       return false;
>>>>      case IFN_MASK_STORE_LANES:
>>>>      case IFN_MASK_LEN_STORE_LANES:
>>>> +      case IFN_ACCESS_WITH_SIZE:
>>> 
>>> below you make it not store, so grouping with store IFNs is a bit odd?
>>> What's wrong with
>>> going through default: for IFN_ACCESS_WITH_SIZE?  Does .ACCESS_WITH_SIZE
>>> perform a read?  How's that represented?
>> Yes, it reads from the 2nd argument when used by __builtin_dynamic_object_size, or c-ubsan,  etc.
>> 
>> .ACCESS_WITH_SIZE (REF_TO_OBJ, REF_TO_SIZE, CLASS_OF_SIZE, SIZE_OF_SIZE, ACCESS_MODE, INDEX)
>> 
>> The 2nd argument is the reference to the size of the object (which is the field that holds the “counted-by” value in the structure).
>> When __builtin_dynamic_object_size queries the size of the object, GCC will generate a MEM_REF to the 2nd argument to get the size of the object, please see the routine “access_with_size_object_size” in gcc/tree-object-size.cc <http://tree-object-size.cc/>.
> 
> But "process_args" doesn't consider that - that only considers aggregate uses.
> Your change handles the call like if it were CONST.  The default handling should
> be fine here, you shouldn't need to handle .ACCESS_WITH_SIZE in
> tree-ssa-alias.cc at all
> (if you make it PURE).

Yes, marking it as PURE will make the implementation simpler. 

Thanks a lot.

Will update this.

Qing

> 
>>> 
>>>>       goto process_a
>>>>      case IFN_MASK_LOAD:
>>>>      case IFN_LEN_LOAD:
>>>> @@ -3073,6 +3074,7 @@ call_may_clobber_ref_p_1 (gcall *call, ao_ref *ref, bool tbaa_p)
>>>>      case IFN_UBSAN_OBJECT_SIZE:
>>>>      case IFN_UBSAN_PTR:
>>>>      case IFN_ASAN_CHECK:
>>>> +      case IFN_ACCESS_WITH_SIZE:
>>> 
>>> it doesn't store.  So above it should be ECF_PURE at least.
>> 
>> As summarized in https://gcc.gnu.org/pipermail/gcc-patches/2023-November/635158.html
>> 
>> ACCESS_WITH_SIZE is not PURE anymore, but it’s only read from the 2nd argument, and not modify anything in the pointed objects.
> 
> I think it is.
> 
>> Therefore, I think the change in the above is correct.
>> 
>> i.e:
>>        /* Treat these internal calls like ECF_PURE for aliasing,
>>           they don't write to any memory the program should care about.
>>           They have important other side-effects, and read memory,
>>           so can't be ECF_NOVOPS.  */
>>      case IFN_UBSAN_NULL:
>>      case IFN_UBSAN_BOUNDS:
>>      case IFN_UBSAN_VPTR:
>>      case IFN_UBSAN_OBJECT_SIZE:
>>      case IFN_UBSAN_PTR:
>>      case IFN_ASAN_CHECK:
>>      case IFN_ACCESS_WITH_SIZE:
>>        return false;
>> 
>> What do you think?
>> 
>> 
>>>>       return false;
>>>>      case IFN_MASK_STORE:
>>>>      case IFN_LEN_STORE:
>>>> diff --git a/gcc/tree-ssa-dce.cc b/gcc/tree-ssa-dce.cc
>>>> index 636c471d4c89..a54fb1b754dd 100644
>>>> --- a/gcc/tree-ssa-dce.cc
>>>> +++ b/gcc/tree-ssa-dce.cc
>>>> @@ -1459,8 +1459,8 @@ eliminate_unnecessary_stmts (bool aggressive)
>>>>                 update_stmt (stmt);
>>>>                 release_ssa_name (name);
>>>> 
>>>> -                 /* GOMP_SIMD_LANE (unless three argument) or ASAN_POISON
>>>> -                    without lhs is not needed.  */
>>>> +                 /* GOMP_SIMD_LANE (unless three argument), ASAN_POISON
>>>> +                    or .ACCESS_WITH_SIZE without lhs is not needed.  */
>>>>                 if (gimple_call_internal_p (stmt))
>>>>                   switch (gimple_call_internal_fn (stmt))
>>>>                     {
>>>> @@ -1470,6 +1470,7 @@ eliminate_unnecessary_stmts (bool aggressive)
>>>>                         break;
>>>>                       /* FALLTHRU */
>>>>                     case IFN_ASAN_POISON:
>>>> +                     case IFN_ACCESS_WITH_SIZE:
>>> 
>>> this shouldn't be necessary if you make .ACCESS_WITH_SIZE ECF_PURE
>>> (or ECF_CONST if it also doesn't read from memory)
>> As explained in the above, .ACCESS_WITH_SIZE is not PURE.
>>> 
>>>>                       remove_dead_stmt (&gsi, bb, to_remove_edges);
>>>>                       break;
>>>>                     default:
>>>> diff --git a/gcc/tree.cc b/gcc/tree.cc
>>>> index 3dff8c510832..5fdb425f612a 100644
>>>> --- a/gcc/tree.cc
>>>> +++ b/gcc/tree.cc
>>>> @@ -4068,7 +4068,8 @@ process_call_operands (tree t)
>>>>  int i = call_expr_flags (t);
>>>> 
>>>>  /* Calls have side-effects, except those to const or pure functions.  */
>>>> -  if ((i & ECF_LOOPING_CONST_OR_PURE) || !(i & (ECF_CONST | ECF_PURE)))
>>>> +  if ((i & ECF_LOOPING_CONST_OR_PURE)
>>>> +      || (!(i & (ECF_CONST | ECF_PURE)) && !is_access_with_size_p (t)))
>>> 
>>> Err, so why not make it ECF_PURE at least?
>> Same above.
>>> 
>>>>    side_effects = true;
>>>>  /* Propagate TREE_READONLY of arguments for const functions.  */
>>>>  if (i & ECF_CONST)
>>>> @@ -13362,6 +13363,28 @@ component_ref_size (tree ref, special_array_member *sam /* = NULL */)
>>>>         ? NULL_TREE : size_zero_node);
>>>> }
>>>> 
>>>> +/* Return true if the given node CALL is a call to a .ACCESS_WITH_SIZE
>>>> +   function.  */
>>>> +bool
>>>> +is_access_with_size_p (const_tree call)
>>>> +{
>>>> +  if (TREE_CODE (call) != CALL_EXPR)
>>>> +    return false;
>>>> +  if (CALL_EXPR_IFN (call) == IFN_ACCESS_WITH_SIZE)
>>>> +    return true;
>>>> +  return false;
>>>> +}
>>>> +
>>>> +/* Get the corresponding reference from the call to a .ACCESS_WITH_SIZE.
>>>> + * i.e the first argument of this call.  Return NULL_TREE otherwise.  */
>>>> +tree
>>>> +get_ref_from_access_with_size (tree call)
>>>> +{
>>>> +  if (is_access_with_size_p (call))
>>>> +    return  CALL_EXPR_ARG (call, 0);
>>>> +  return NULL_TREE;
>>>> +}
>>>> +
>>>> /* Return the machine mode of T.  For vectors, returns the mode of the
>>>>   inner type.  The main use case is to feed the result to HONOR_NANS,
>>>>   avoiding the BLKmode that a direct TYPE_MODE (T) might return.  */
>>>> diff --git a/gcc/tree.h b/gcc/tree.h
>>>> index 972a067a1f7a..fbaef3e5fb5c 100644
>>>> --- a/gcc/tree.h
>>>> +++ b/gcc/tree.h
>>>> @@ -5760,6 +5760,14 @@ extern special_array_member component_ref_sam_type (tree);
>>>>   cannot be determined.  */
>>>> extern tree component_ref_size (tree, special_array_member * = NULL);
>>>> 
>>>> +/* Return true if the given node is a call to a .ACCESS_WITH_SIZE
>>>> +   function.  */
>>>> +extern bool is_access_with_size_p (const_tree);
>>>> +
>>>> +/* Get the corresponding reference from the call to a .ACCESS_WITH_SIZE,
>>>> + * i.e. the first argument of this call.  Return NULL_TREE otherwise.  */
>>>> +extern tree get_ref_from_access_with_size (tree);
>>>> +
>>>> extern int tree_map_base_eq (const void *, const void *);
>>>> extern unsigned int tree_map_base_hash (const void *);
>>>> extern bool tree_map_base_marked_p (const void *);
>>>> diff --git a/gcc/varasm.cc b/gcc/varasm.cc
>>>> index fa17eff551e8..d75b23668925 100644
>>>> --- a/gcc/varasm.cc
>>>> +++ b/gcc/varasm.cc
>>>> @@ -5082,6 +5082,11 @@ initializer_constant_valid_p_1 (tree value, tree endtype, tree *cache)
>>>>       }
>>>>      return ret;
>>>> 
>>>> +    case CALL_EXPR:
>>>> +      /* For a call to .ACCESS_WITH_SIZE, check the first argument.  */
>>>> +      if (tree ref = get_ref_from_access_with_size (value))
>>>> +       return initializer_constant_valid_p_1 (ref, endtype, cache);
>>> 
>>> I think we should fold/strip .ACCESS_WITH_SIZE from initializers
>>> instead.  That would be
>>> the frontends job I guess, most probably not even generate those in
>>> the first place?
>> 
>> Sounds reasonable, I will see how to do this in C FE.
>> Joseph, do you have any suggestion where in C FE I should do this folding?
>> 
>> thanks.
>> 
>> Qing
>>> 
>>>> +      /* FALLTHROUGH.  */
>>>>    default:
>>>>      break;
>>>>    }
>>>> @@ -5276,6 +5281,11 @@ output_constant (tree exp, unsigned HOST_WIDE_INT size, unsigned int align,
>>>>       exp = TREE_OPERAND (exp, 0);
>>>>    }
>>>> 
>>>> +  /* For a call to .ACCESS_WITH_SIZE, check the first argument.  */
>>>> +  if (TREE_CODE (exp) == CALL_EXPR)
>>>> +    if (tree ref = get_ref_from_access_with_size (exp))
>>>> +      exp = ref;
>>>> +
>>>>  code = TREE_CODE (TREE_TYPE (exp));
>>>>  thissize = int_size_in_bytes (TREE_TYPE (exp));
>>>> 
>>>> --
>>>> 2.31.1