Re: [patch][version 6] add -ftrivial-auto-var-init and variable attribute "uninitialized" to gcc

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

> On Aug 16, 2021, at 2:40 AM, Richard Biener <rguenther@suse.de> wrote:
> 
> On Thu, 12 Aug 2021, Qing Zhao wrote:
> 
>> Hi, Richard,
>> 
>> For RTL expansion of call to .DEFERRED_INIT, I changed my code per your suggestions like following:
>> 
>> ======================
>> #define INIT_PATTERN_VALUE  0xFE
>> static void
>> expand_DEFERRED_INIT (internal_fn, gcall *stmt)
>> {
>>  tree lhs = gimple_call_lhs (stmt);
>>  tree var_size = gimple_call_arg (stmt, 0);
>>  enum auto_init_type init_type
>>    = (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
>>  bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));
>> 
>>  tree var_type = TREE_TYPE (lhs);
>>  gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);
>> 
>>  if (is_vla || (!can_native_interpret_type_p (var_type)))
>>    {
>>    /* If this is a VLA or the type of the variable cannot be natively
>>       interpreted, expand to a memset to initialize it.  */
>>      if (TREE_CODE (lhs) == SSA_NAME)
>>        lhs = SSA_NAME_VAR (lhs);
>>      tree var_addr = NULL_TREE;
>>      if (is_vla)
>>        var_addr = TREE_OPERAND (lhs, 0);
>>      else
>>        {
>>         TREE_ADDRESSABLE (lhs) = 1;
>>         var_addr = build_fold_addr_expr (lhs);
>>        }
>>      tree value = (init_type == AUTO_INIT_PATTERN) ?
>>                    build_int_cst (unsigned_char_type_node,
>>                                   INIT_PATTERN_VALUE) :
>>                    build_zero_cst (unsigned_char_type_node);
>>      tree m_call = build_call_expr (builtin_decl_implicit (BUILT_IN_MEMSET),
>>                                     3, var_addr, value, var_size);
>>      /* Expand this memset call.  */
>>      expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
>>    }
>>  else
>>    {
>>    /* If this is not a VLA and the type of the variable can be natively 
>>       interpreted, expand to assignment to generate better code.  */
>>      tree pattern = NULL_TREE;
>>      unsigned HOST_WIDE_INT total_bytes
>>        = tree_to_uhwi (TYPE_SIZE_UNIT (var_type));
>> 
>>      if (init_type == AUTO_INIT_PATTERN)
>>        {
>>          unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
>>          memset (buf, INIT_PATTERN_VALUE, total_bytes);
>>          pattern = native_interpret_expr (var_type, buf, total_bytes);
>>          gcc_assert (pattern);
>>        }
>> 
>>      tree init = (init_type == AUTO_INIT_PATTERN) ?
>>                   pattern :
>>                   build_zero_cst (var_type);
>>      expand_assignment (lhs, init, false);
>>    }
>> }
>> ===========================
>> 
>> Now, I used “can_native_interpret_type_p (var_type)” instead of “use_register_for_decl (lhs)” to decide 
>> whether to use “memset” or use “assign” to expand this function.
>> 
>> However, this exposed an bug that is very hard to be addressed:
>> 
>> *******For the testing case: test suite/gcc.dg/uninit-I.c:
>> 
>> /* { dg-do compile } */
>> /* { dg-options "-O2 -Wuninitialized" } */
>> 
>> int sys_msgctl (void)
>> {
>>  struct { int mode; } setbuf;
>>  return setbuf.mode;  /* { dg-warning "'setbuf\.mode' is used" } */
>> ==
>> 
>> ******the above auto var “setbuf” has “struct” type, which “can_native_interpret_type_p(var_type)” is false, therefore, 
>> Expanding this .DEFERRED_INIT call went down the “memset” expansion route. 
>> 
>> However, this structure type can be fitted into a register, therefore cannot be taken address anymore at this stage, even though I tried:
>> 
>>         TREE_ADDRESSABLE (lhs) = 1;
>>         var_addr = build_fold_addr_expr (lhs);
>> 
>> To create an address variable for it, the expansion still failed at expr.c: line 8412:
>> during RTL pass: expand
>> /home/opc/Work/GCC/latest-gcc/gcc/testsuite/gcc.dg/auto-init-uninit-I.c:6:24: internal compiler error: in expand_expr_addr_expr_1, at expr.c:8412
>> 0xd04104 expand_expr_addr_expr_1
>> 	../../latest-gcc/gcc/expr.c:8412
>> 0xd04a95 expand_expr_addr_expr
>> 	../../latest-gcc/gcc/expr.c:8525
>> 0xd13592 expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
>> 	../../latest-gcc/gcc/expr.c:11741
>> 0xd05142 expand_expr_real(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
>> 	../../latest-gcc/gcc/expr.c:8713
>> 0xaed1d3 expand_expr
>> 	../../latest-gcc/gcc/expr.h:301
>> 0xaf0d89 get_memory_rtx
>> 	../../latest-gcc/gcc/builtins.c:1370
>> 0xafb4fb expand_builtin_memset_args
>> 	../../latest-gcc/gcc/builtins.c:4102
>> 0xafacde expand_builtin_memset(tree_node*, rtx_def*, machine_mode)
>> 	../../latest-gcc/gcc/builtins.c:3886
>> 0xe97fb3 expand_DEFERRED_INIT
>> 
>> ******That’s the major reason why I chose “use_register_for_decl(lhs)” to decide “memset” expansion or “assign” expansion, “memset” expansion
>> needs to take address of the variable, if the variable has been decided to fit into a register, then its address cannot taken anymore at this stage.
>> 
>> ******using “can_native_interpret_type_p” did make the “pattern” generation part much  cleaner and simpler, however, looks like it didn’t work correctly.
>> 
>> Based on this, I’d like to keep my previous implementation by using “use_register_for_decl” to decide whether to take “memset” expansion or “assign” expansion.
>> Therefore, I might still need to keep the “UGLY”  implementation of generatting “pattern” constant for different types?
>> 
>> Let me know your opinion on this.
> 
> Hmm, I think you can use use_register_for_decl(lhs) to decide to use an
> alternate type to generate the pattern (and feed to 
> can_native_interpret_type_p) by using
> lang_hooks.type_for_mode (TYPE_MODE (TREE_TYPE (lhs))).  You can then
> build the assignment from the pattern as
> 
> VIEW_CONVERT <reg-type> (lhs) = pattern_cst;
> 
> note that more RTL-expand-ish would be to simply expand 'lhs' and
> see whether it's a REG_P or a MEM_P and decide based on that.  Of course
> that no longer allows you to use the high-level expand_assignment
> or memset but you'd need to work on a lower level then.
> 
> Richard.


My current code for expand_DEFERRED_INIT is like the following, could you check and see whether there is any issue for it:

#define INIT_PATTERN_VALUE  0xFE
static void
expand_DEFERRED_INIT (internal_fn, gcall *stmt)
{
  tree lhs = gimple_call_lhs (stmt);
  tree var_size = gimple_call_arg (stmt, 0);
  enum auto_init_type init_type
    = (enum auto_init_type) TREE_INT_CST_LOW (gimple_call_arg (stmt, 1));
  bool is_vla = (bool) TREE_INT_CST_LOW (gimple_call_arg (stmt, 2));

  tree var_type = TREE_TYPE (lhs);
  gcc_assert (init_type > AUTO_INIT_UNINITIALIZED);

  if (is_vla || (!use_register_for_decl (lhs)))
    {
      if (TREE_CODE (lhs) == SSA_NAME)
        lhs = SSA_NAME_VAR (lhs);
    /* If this is a VLA or the variable is not in register,
       expand to a memset to initialize it.  */
      tree var_addr = NULL_TREE;
      if (is_vla)
        var_addr = TREE_OPERAND (lhs, 0);
      else
        {
          TREE_ADDRESSABLE (lhs) = 1;
          var_addr = build_fold_addr_expr (lhs);
        }

      tree value = (init_type == AUTO_INIT_PATTERN) ?
                    build_int_cst (unsigned_char_type_node,
                                   INIT_PATTERN_VALUE) :
                    build_zero_cst (unsigned_char_type_node);
      tree m_call = build_call_expr (builtin_decl_implicit (BUILT_IN_MEMSET),
                                     3, var_addr, value, var_size);
      /* Expand this memset call.  */
      expand_builtin_memset (m_call, NULL_RTX, TYPE_MODE (var_type));
    }
  else
    {
    /* If this variable is in a register, use expand_assignment might
       generate better code.  */
      tree pattern = NULL_TREE;
      unsigned HOST_WIDE_INT total_bytes
        = tree_to_uhwi (TYPE_SIZE_UNIT (var_type));

      if (init_type == AUTO_INIT_PATTERN)
        {
          if (can_native_interpret_type_p (var_type))
            {
              unsigned char *buf = (unsigned char *) xmalloc (total_bytes);
              memset (buf, INIT_PATTERN_VALUE, total_bytes);
              pattern = native_interpret_expr (var_type, buf, total_bytes);
              gcc_assert (pattern);
            }
          else
            {
              tree index_type = build_index_type (size_int (total_bytes - 1));
              tree array_type = build_array_type (unsigned_char_type_node,
                                                  index_type);
              tree element = build_int_cst (unsigned_char_type_node,
                                            INIT_PATTERN_VALUE);
              vec<constructor_elt, va_gc> *elts = NULL;
              for (unsigned int i = 0; i < total_bytes; i++)
                CONSTRUCTOR_APPEND_ELT (elts, NULL_TREE, element);
              pattern = build_constructor (array_type, elts);
              pattern = build1 (VIEW_CONVERT_EXPR, var_type, pattern);
            }
        }

      tree init = (init_type == AUTO_INIT_PATTERN) ?
                   pattern :
                   build_zero_cst (var_type);
      expand_assignment (lhs, init, false);
    }
}

Thanks.

Qing