This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix up handling of not really variable VLAs (PR libgomp/83590)


On Fri, Jan 12, 2018 at 05:45:15PM +0100, Richard Biener wrote:
> >In a few C testcases the cases with TREE_CONSTANT before gimplify_expr
> >are
> >stuff like:
> >extern void dynreplace_trampoline(void);
> >extern void dynreplace_trampoline_endlabel(void);
> >int dynreplace_add_trampoline(void)
> >{
> >unsigned long trampoline_code[(((unsigned long)
> >(&(dynreplace_trampoline_endlabel))-(unsigned long)
> >(&dynreplace_trampoline)))];
> >}
> >and similar, for C is_gimple_constant after, i.e. what the patch
> >changes, is
> >those VLAs that use const vars in the expressions.  Changing that is
> >perhaps
> >acceptable.  On the other side, the amount of Ada changes is
> >significantly
> >higher.  Perhaps do
> >  if (is_gimple_constant (*expr_p)
> >      && (flag_openmp || flag_openacc || flag_openmp_simd))
> >, that way it will never affect Ada.
> 
> So what about not doing the temp var if the expr was TREE_CONSTANT? 

It is true that in my log file I don't have any cases where in C
it was first TREE_CONSTANT and turned into is_gimple_constant, but that
doesn't mean it isn't possible.

Looking at a random testcase, it is
#0  gimplify_one_sizepos(tree_node**, gimple**) () at ../../gcc/gimplify.c:12572
#1  0x000000000097f78d in gnat_gimplify_expr(tree_node**, gimple**, gimple**) () at ../../gcc/ada/gcc-interface/trans.c:8465
#2  0x0000000000ecf638 in gimplify_expr(tree_node**, gimple**, gimple**, bool (*)(tree_node*), int) () at ../../gcc/gimplify.c:11343
#3  0x0000000000ed4087 in gimplify_stmt (stmt_p=<optimized out>, seq_p=seq_p@entry=0x7fffffffd880) at ../../gcc/gimplify.c:6658
#4  0x0000000000ed0f9c in gimplify_statement_list (pre_p=0x7fffffffd880, expr_p=0x7fffef8fd380) at ../../gcc/tree-iterator.h:86
which is:
      /* The expressions for the RM bounds must be gimplified to ensure that
         they are properly elaborated.  See gimplify_decl_expr.  */
      if ((TREE_CODE (op) == TYPE_DECL || TREE_CODE (op) == VAR_DECL)
          && !TYPE_SIZES_GIMPLIFIED (TREE_TYPE (op)))
        switch (TREE_CODE (TREE_TYPE (op)))
          {
          case INTEGER_TYPE:
          case ENUMERAL_TYPE:
          case BOOLEAN_TYPE:
          case REAL_TYPE:
            {
              tree type = TYPE_MAIN_VARIANT (TREE_TYPE (op)), t, val;

              val = TYPE_RM_MIN_VALUE (type);
              if (val)
                {
                  gimplify_one_sizepos (&val, pre_p);
                  for (t = type; t; t = TYPE_NEXT_VARIANT (t))
                    SET_TYPE_RM_MIN_VALUE (t, val);
                }

              val = TYPE_RM_MAX_VALUE (type);
              if (val)
                {
                  gimplify_one_sizepos (&val, pre_p);
                  for (t = type; t; t = TYPE_NEXT_VARIANT (t))
                    SET_TYPE_RM_MAX_VALUE (t, val);
                }

            }
            break;

          default:
            break;
          }
Those are the only 2 calls of gimplify_one_sizepos in the Ada FE,
so perhaps we could just use a different function for this purpose instead?

	Jakub


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]