This is the mail archive of the
gcc@gcc.gnu.org
mailing list for the GCC project.
type gimplification, continued
- From: Richard Henderson <rth at redhat dot com>
- To: Richard Kenner <kenner at vlsi1 dot ultra dot nyu dot edu>
- Cc: gcc at gcc dot gnu dot org
- Date: Wed, 22 Dec 2004 00:42:28 -0800
- Subject: type gimplification, continued
- References: <10412212229.AA01178@vlsi1.ultra.nyu.edu>
Kenner, I still think you're wrong.
Here's all of my patches reverted, plus the patch you gave
me yesterday. It does pretty good, in that it bootstraps.
I think what you tested was *not* all of my patches reverted,
which would be cheating. We shouldn't need *both* unsharing
and TYPE_SIZES_GIMPLIFIED.
But it fails gcc.c-torture/compile/nested-1.c, which again
shows invalid tree sharing between the parent and the nested
function. When we go to gimplify the DECL_SIZE of the
nested function parameter, we simultaneously modify one of
the statements in the parent function, which means that a
variable local to the nested function leaks into the parent.
There is, perhaps, an alternate solution to adding SAVE_EXPRs
everywhere. In gimplify_one_sizepos, do
*expr_p = unshare_expr (*expr_p);
gimplify_expr (expr_p, stmt_p, NULL, is_gimple_val, fb_rvalue);
which will make sure that any bits of the expression that we
insert into the statement list for the current function won't
be shared with some other function. We're still storing the
final result variable into the type node, so we would still
need to protect against multiple gimplifications of the type
node.
I'm not sure what that buys us in terms of reliability though.
r~
Index: function.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/function.c,v
retrieving revision 1.594
diff -c -p -d -r1.594 function.c
*** function.c 21 Dec 2004 01:37:31 -0000 1.594
--- function.c 22 Dec 2004 08:29:41 -0000
*************** gimplify_parm_type (tree *tp, int *walk_
*** 3172,3178 ****
{
if (POINTER_TYPE_P (t))
*walk_subtrees = 1;
! else if (TYPE_SIZE (t) && !TREE_CONSTANT (TYPE_SIZE (t)))
{
gimplify_type_sizes (t, (tree *) data);
*walk_subtrees = 1;
--- 3172,3179 ----
{
if (POINTER_TYPE_P (t))
*walk_subtrees = 1;
! else if (TYPE_SIZE (t) && !TREE_CONSTANT (TYPE_SIZE (t))
! && !TYPE_SIZES_GIMPLIFIED (t))
{
gimplify_type_sizes (t, (tree *) data);
*walk_subtrees = 1;
Index: gimplify.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/gimplify.c,v
retrieving revision 2.95
diff -c -p -d -r2.95 gimplify.c
*** gimplify.c 21 Dec 2004 19:27:07 -0000 2.95
--- gimplify.c 22 Dec 2004 08:29:41 -0000
*************** gimplify_decl_expr (tree *stmt_p)
*** 996,1002 ****
of the emitted code: see mx_register_decls(). */
tree t, args, addr, ptr_type;
! gimplify_type_sizes (TREE_TYPE (decl), stmt_p);
gimplify_one_sizepos (&DECL_SIZE (decl), stmt_p);
gimplify_one_sizepos (&DECL_SIZE_UNIT (decl), stmt_p);
--- 996,1007 ----
of the emitted code: see mx_register_decls(). */
tree t, args, addr, ptr_type;
! /* ??? We really shouldn't need to gimplify the type of the variable
! since it already should have been done. But leave this here
! for now to avoid disrupting too many things at once. */
! if (!TYPE_SIZES_GIMPLIFIED (TREE_TYPE (decl)))
! gimplify_type_sizes (TREE_TYPE (decl), stmt_p);
!
gimplify_one_sizepos (&DECL_SIZE (decl), stmt_p);
gimplify_one_sizepos (&DECL_SIZE_UNIT (decl), stmt_p);
*************** gimplify_expr (tree *expr_p, tree *pre_p
*** 4180,4186 ****
void
gimplify_type_sizes (tree type, tree *list_p)
{
! tree field;
switch (TREE_CODE (type))
{
--- 4185,4201 ----
void
gimplify_type_sizes (tree type, tree *list_p)
{
! tree field, t;
!
! /* Note that we do not check for TYPE_SIZES_GIMPLIFIED already set because
! that's not supposed to happen on types where gimplifcation does anything.
! We should assert that it isn't set, but we can indeed be called multiple
! times on pointers. Unfortunately, this includes fat pointers which we
! can't easily test for. We could pass TYPE down to gimplify_one_sizepos
! and test there, but it doesn't seem worth it. */
!
! /* We first do the main variant, then copy into any other variants. */
! type = TYPE_MAIN_VARIANT (type);
switch (TREE_CODE (type))
{
*************** gimplify_type_sizes (tree type, tree *li
*** 4194,4204 ****
case REAL_TYPE:
gimplify_one_sizepos (&TYPE_MIN_VALUE (type), list_p);
gimplify_one_sizepos (&TYPE_MAX_VALUE (type), list_p);
break;
case ARRAY_TYPE:
! /* These anonymous types don't have declarations, so handle them here. */
! gimplify_type_sizes (TYPE_DOMAIN (type), list_p);
break;
case RECORD_TYPE:
--- 4209,4230 ----
case REAL_TYPE:
gimplify_one_sizepos (&TYPE_MIN_VALUE (type), list_p);
gimplify_one_sizepos (&TYPE_MAX_VALUE (type), list_p);
+
+ for (t = TYPE_NEXT_VARIANT (type); t; t = TYPE_NEXT_VARIANT (t))
+ {
+ TYPE_MIN_VALUE (t) = TYPE_MIN_VALUE (type);
+ TYPE_MAX_VALUE (t) = TYPE_MAX_VALUE (type);
+ TYPE_SIZES_GIMPLIFIED (t) = 1;
+ }
break;
case ARRAY_TYPE:
! /* These types may not have declarations, so handle them here. */
! if (!TYPE_SIZES_GIMPLIFIED (TREE_TYPE (type)))
! gimplify_type_sizes (TREE_TYPE (type), list_p);
!
! if (!TYPE_SIZES_GIMPLIFIED (TYPE_DOMAIN (type)))
! gimplify_type_sizes (TYPE_DOMAIN (type), list_p);
break;
case RECORD_TYPE:
*************** gimplify_type_sizes (tree type, tree *li
*** 4215,4237 ****
gimplify_one_sizepos (&TYPE_SIZE (type), list_p);
gimplify_one_sizepos (&TYPE_SIZE_UNIT (type), list_p);
- }
! /* A subroutine of gimplify_one_sizepos, called via walk_tree. Evaluate
! the expression if it's a SAVE_EXPR and add it to the statement list
! in DATA. */
!
! static tree
! eval_save_expr (tree *tp, int *walk_subtrees, void *data)
! {
! if (TREE_CODE (*tp) == SAVE_EXPR)
{
! *walk_subtrees = 0;
! gimplify_and_add (*tp, (tree *) data);
}
! else if (TYPE_P (*tp) || DECL_P (*tp))
! *walk_subtrees = 0;
! return NULL;
}
/* A subroutine of gimplify_type_sizes to make sure that *EXPR_P,
--- 4241,4255 ----
gimplify_one_sizepos (&TYPE_SIZE (type), list_p);
gimplify_one_sizepos (&TYPE_SIZE_UNIT (type), list_p);
! for (t = TYPE_NEXT_VARIANT (type); t; t = TYPE_NEXT_VARIANT (t))
{
! TYPE_SIZE (t) = TYPE_SIZE (type);
! TYPE_SIZE_UNIT (t) = TYPE_SIZE_UNIT (type);
! TYPE_SIZES_GIMPLIFIED (t) = 1;
}
!
! TYPE_SIZES_GIMPLIFIED (type) = 1;
}
/* A subroutine of gimplify_type_sizes to make sure that *EXPR_P,
*************** gimplify_one_sizepos (tree *expr_p, tree
*** 4251,4257 ****
|| CONTAINS_PLACEHOLDER_P (*expr_p))
return;
! walk_tree (expr_p, eval_save_expr, stmt_p, NULL);
}
#ifdef ENABLE_CHECKING
--- 4269,4275 ----
|| CONTAINS_PLACEHOLDER_P (*expr_p))
return;
! gimplify_expr (expr_p, stmt_p, NULL, is_gimple_val, fb_rvalue);
}
#ifdef ENABLE_CHECKING
Index: stor-layout.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/stor-layout.c,v
retrieving revision 1.222
diff -c -p -d -r1.222 stor-layout.c
*** stor-layout.c 21 Dec 2004 19:27:08 -0000 1.222
--- stor-layout.c 22 Dec 2004 08:29:42 -0000
*************** variable_size (tree size)
*** 125,143 ****
just return SIZE unchanged. Likewise for self-referential sizes and
constant sizes. */
if (TREE_CONSTANT (size)
- || TREE_CODE (size) == SAVE_EXPR
|| lang_hooks.decls.global_bindings_p () < 0
|| CONTAINS_PLACEHOLDER_P (size))
return size;
! /* Force creation of a SAVE_EXPR. This solves (1) code duplication
! problems between parent and nested functions that occasionally can't
! be cleaned up because of portions of the expression escaping the
! parent function via the FRAME object, and (2) tree sharing problems
! between the type system and the gimple code, which can leak SSA_NAME
! objects into e.g. TYPE_SIZE, which cause heartburn when emitting
! debug information. */
! size = build1 (SAVE_EXPR, TREE_TYPE (size), size);
/* If an array with a variable number of elements is declared, and
the elements require destruction, we will emit a cleanup for the
--- 125,135 ----
just return SIZE unchanged. Likewise for self-referential sizes and
constant sizes. */
if (TREE_CONSTANT (size)
|| lang_hooks.decls.global_bindings_p () < 0
|| CONTAINS_PLACEHOLDER_P (size))
return size;
! size = save_expr (size);
/* If an array with a variable number of elements is declared, and
the elements require destruction, we will emit a cleanup for the
*************** layout_decl (tree decl, unsigned int kno
*** 333,340 ****
if (DECL_SIZE (decl) == 0)
{
! DECL_SIZE (decl) = unshare_expr (TYPE_SIZE (type));
! DECL_SIZE_UNIT (decl) = unshare_expr (TYPE_SIZE_UNIT (type));
}
else if (DECL_SIZE_UNIT (decl) == 0)
DECL_SIZE_UNIT (decl)
--- 325,332 ----
if (DECL_SIZE (decl) == 0)
{
! DECL_SIZE (decl) = TYPE_SIZE (type);
! DECL_SIZE_UNIT (decl) = TYPE_SIZE_UNIT (type);
}
else if (DECL_SIZE_UNIT (decl) == 0)
DECL_SIZE_UNIT (decl)
*************** layout_type (tree type)
*** 1644,1651 ****
if (index && TYPE_MAX_VALUE (index) && TYPE_MIN_VALUE (index)
&& TYPE_SIZE (element))
{
! tree ub = unshare_expr (TYPE_MAX_VALUE (index));
! tree lb = unshare_expr (TYPE_MIN_VALUE (index));
tree length;
tree element_size;
--- 1636,1643 ----
if (index && TYPE_MAX_VALUE (index) && TYPE_MIN_VALUE (index)
&& TYPE_SIZE (element))
{
! tree ub = TYPE_MAX_VALUE (index);
! tree lb = TYPE_MIN_VALUE (index);
tree length;
tree element_size;
Index: tree-sra.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree-sra.c,v
retrieving revision 2.47
diff -c -p -d -r2.47 tree-sra.c
*** tree-sra.c 10 Dec 2004 21:54:42 -0000 2.47
--- tree-sra.c 22 Dec 2004 08:29:42 -0000
*************** type_can_be_decomposed_p (tree type)
*** 184,190 ****
return false;
/* The type must have a definite nonzero size. */
! if (TYPE_SIZE (type) == NULL || integer_zerop (TYPE_SIZE (type)))
goto fail;
/* The type must be a non-union aggregate. */
--- 184,191 ----
return false;
/* The type must have a definite nonzero size. */
! if (TYPE_SIZE (type) == NULL || TREE_CODE (TYPE_SIZE (type)) != INTEGER_CST
! || integer_zerop (TYPE_SIZE (type)))
goto fail;
/* The type must be a non-union aggregate. */
Index: tree.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/tree.h,v
retrieving revision 1.669
diff -c -p -d -r1.669 tree.h
*** tree.h 21 Dec 2004 17:43:11 -0000 1.669
--- tree.h 22 Dec 2004 08:29:43 -0000
*************** struct tree_common GTY(())
*** 368,373 ****
--- 368,375 ----
all expressions
all decls
all constants
+ TYPE_SIZES_GIMPLIFIED
+ ..._TYPE
unsigned_flag:
*************** extern void tree_operand_check_failed (i
*** 934,939 ****
--- 936,944 ----
also appear in an expression or decl where the value is constant. */
#define TREE_CONSTANT(NODE) (NON_TYPE_CHECK (NODE)->common.constant_flag)
+ /* Nonzero if NODE, a type, has had its sizes gimplified. */
+ #define TYPE_SIZES_GIMPLIFIED(NODE) (TYPE_CHECK (NODE)->common.constant_flag)
+
/* In a decl (most significantly a FIELD_DECL), means an unsigned field. */
#define DECL_UNSIGNED(NODE) (DECL_CHECK (NODE)->common.unsigned_flag)
Index: ada/trans.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/ada/trans.c,v
retrieving revision 1.85
diff -c -p -d -r1.85 trans.c
*** ada/trans.c 19 Dec 2004 19:59:23 -0000 1.85
--- ada/trans.c 22 Dec 2004 08:29:45 -0000
*************** add_decl_expr (tree gnu_decl, Entity_Id
*** 4283,4291 ****
}
}
! /* Utility function to mark nodes with TREE_VISITED. Called from walk_tree.
! We use this to indicate all variable sizes and positions in global types
! may not be shared by any subprogram. */
static tree
mark_visited (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED)
--- 4283,4292 ----
}
}
! /* Utility function to mark nodes with TREE_VISITED and types as having their
! sized gimplified. Called from walk_tree. We use this to indicate all
! variable sizes and positions in global types may not be shared by any
! subprogram. */
static tree
mark_visited (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED)
*************** mark_visited (tree *tp, int *walk_subtre
*** 4298,4303 ****
--- 4299,4307 ----
else if (!TYPE_IS_DUMMY_P (*tp))
TREE_VISITED (*tp) = 1;
+ if (TYPE_P (*tp))
+ TYPE_SIZES_GIMPLIFIED (*tp) = 1;
+
return NULL_TREE;
}