From c597ef4eab9a2de9ad0b2187547ac9bac0b53132 Mon Sep 17 00:00:00 2001 From: Diego Novillo Date: Thu, 12 Aug 2004 14:34:11 +0000 Subject: [PATCH] re PR tree-optimization/16867 (Inline array initializer miscompilation at -O) PR tree-optimization/16867 * tree.c (is_global_var): New function. (needs_to_live_in_memory): Check for TREE_ADDRESSABLE. Call is_global_var. * tree.h (DECL_NEEDS_TO_LIVE_IN_MEMORY_INTERNAL): Remove. Update all users. (is_global_var): Declare. * tree-dfa.c (dump_variable): Display global and addressable attributes. (add_referenced_var): Clarify documentation when marking variables call-clobbered. * tree-flow-inline.h (is_call_clobbered): Call is_global_var instead of needs_to_live_in_memory. (mark_call_clobbered): If the variable is a tag, mark it DECL_EXTERNAL. * tree-gimple.c (is_gimple_reg): Don't check for TREE_ADDRESSABLE. (is_gimple_non_addressable): Likewise. * tree-ssa-alias.c (get_nmt_for): Always check whether the tag needs to be marked call-clobbered. (setup_pointers_and_addressables): Call is_global_var instead of needs_to_live_in_memory. * tree-ssa-dce.c (need_to_preserve_store): Remove. Update all users with is_global_var. (mark_stmt_if_obviously_necessary): Fix processing of aliased stores. Don't check the virtual definitions. Rather, check whether the store is going into global memory. (find_obviously_necessary_stmts): Get the symbol from the PHI result. * tree-ssa-operands.c (get_call_expr_operands): Do not add clobbering may-defs if the call does not have side effects. libjava/ChangeLog PR tree-optimization/16867 * testsuite/libjava.lang/PR16867.java: New test. From-SVN: r85874 --- gcc/ChangeLog | 34 +++++++ gcc/tree-dfa.c | 11 +- gcc/tree-flow-inline.h | 11 +- gcc/tree-gimple.c | 5 +- gcc/tree-ssa-alias.c | 23 ++--- gcc/tree-ssa-dce.c | 106 ++++++++++++++------ gcc/tree-ssa-operands.c | 4 +- gcc/tree.c | 13 ++- gcc/tree.h | 13 +-- libjava/ChangeLog | 5 + libjava/testsuite/libjava.lang/PR16867.java | 17 ++++ libjava/testsuite/libjava.lang/PR16867.out | 1 + 12 files changed, 173 insertions(+), 70 deletions(-) create mode 100644 libjava/testsuite/libjava.lang/PR16867.java create mode 100644 libjava/testsuite/libjava.lang/PR16867.out diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 4331d9a8b44..b9c2ff04ac1 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,37 @@ +2004-08-12 Diego Novillo + + PR tree-optimization/16867 + * tree.c (is_global_var): New function. + (needs_to_live_in_memory): Check for TREE_ADDRESSABLE. + Call is_global_var. + * tree.h (DECL_NEEDS_TO_LIVE_IN_MEMORY_INTERNAL): Remove. + Update all users. + (is_global_var): Declare. + * tree-dfa.c (dump_variable): Display global and addressable + attributes. + (add_referenced_var): Clarify documentation when marking + variables call-clobbered. + * tree-flow-inline.h (is_call_clobbered): Call is_global_var + instead of needs_to_live_in_memory. + (mark_call_clobbered): If the variable is a tag, mark it + DECL_EXTERNAL. + * tree-gimple.c (is_gimple_reg): Don't check for + TREE_ADDRESSABLE. + (is_gimple_non_addressable): Likewise. + * tree-ssa-alias.c (get_nmt_for): Always check whether the tag + needs to be marked call-clobbered. + (setup_pointers_and_addressables): Call is_global_var instead + of needs_to_live_in_memory. + * tree-ssa-dce.c (need_to_preserve_store): Remove. + Update all users with is_global_var. + (mark_stmt_if_obviously_necessary): Fix processing of aliased + stores. Don't check the virtual definitions. Rather, check + whether the store is going into global memory. + (find_obviously_necessary_stmts): Get the symbol from the PHI + result. + * tree-ssa-operands.c (get_call_expr_operands): Do not add + clobbering may-defs if the call does not have side effects. + 2004-08-12 Jakub Jelinek PR c++/16276 diff --git a/gcc/tree-dfa.c b/gcc/tree-dfa.c index 9e9ee324826..0273406df84 100644 --- a/gcc/tree-dfa.c +++ b/gcc/tree-dfa.c @@ -562,8 +562,11 @@ dump_variable (FILE *file, tree var) if (ann->is_alias_tag) fprintf (file, ", is an alias tag"); - if (needs_to_live_in_memory (var)) - fprintf (file, ", is %s", TREE_STATIC (var) ? "static" : "global"); + if (TREE_ADDRESSABLE (var)) + fprintf (file, ", is addressable"); + + if (is_global_var (var)) + fprintf (file, ", is global"); if (is_call_clobbered (var)) fprintf (file, ", call clobbered"); @@ -900,7 +903,9 @@ add_referenced_var (tree var, struct walk_state *walk_state) v_ann->uid = num_referenced_vars; VARRAY_PUSH_TREE (referenced_vars, var); - /* Global and static variables are call-clobbered, always. */ + /* Initially assume that all memory variables are + call-clobbered. This will be refined later by the alias + analyzer. */ if (needs_to_live_in_memory (var)) mark_call_clobbered (var); diff --git a/gcc/tree-flow-inline.h b/gcc/tree-flow-inline.h index d3e0a32dbad..a9cc21325e8 100644 --- a/gcc/tree-flow-inline.h +++ b/gcc/tree-flow-inline.h @@ -630,7 +630,7 @@ loop_containing_stmt (tree stmt) static inline bool is_call_clobbered (tree var) { - return needs_to_live_in_memory (var) + return is_global_var (var) || bitmap_bit_p (call_clobbered_vars, var_ann (var)->uid); } @@ -639,8 +639,12 @@ static inline void mark_call_clobbered (tree var) { var_ann_t ann = var_ann (var); - /* Call-clobbered variables need to live in memory. */ - DECL_NEEDS_TO_LIVE_IN_MEMORY_INTERNAL (var) = 1; + /* If VAR is a memory tag, then we need to consider it a global + variable. This is because the pointer that VAR represents has + been found to point to either an arbitrary location or to a known + location in global memory. */ + if (ann->mem_tag_kind != NOT_A_TAG) + DECL_EXTERNAL (var) = 1; bitmap_set_bit (call_clobbered_vars, ann->uid); } @@ -649,7 +653,6 @@ static inline void mark_non_addressable (tree var) { bitmap_clear_bit (call_clobbered_vars, var_ann (var)->uid); - DECL_NEEDS_TO_LIVE_IN_MEMORY_INTERNAL (var) = 0; TREE_ADDRESSABLE (var) = 0; } diff --git a/gcc/tree-gimple.c b/gcc/tree-gimple.c index 413395ae531..e2088935515 100644 --- a/gcc/tree-gimple.c +++ b/gcc/tree-gimple.c @@ -441,7 +441,6 @@ is_gimple_reg (tree t) /* A volatile decl is not acceptable because we can't reuse it as needed. We need to copy it into a temp first. */ && ! TREE_THIS_VOLATILE (t) - && ! TREE_ADDRESSABLE (t) && ! needs_to_live_in_memory (t)); } @@ -481,9 +480,7 @@ is_gimple_non_addressable (tree t) if (TREE_CODE (t) == SSA_NAME) t = SSA_NAME_VAR (t); - return (is_gimple_variable (t) - && ! TREE_ADDRESSABLE (t) - && ! needs_to_live_in_memory (t)); + return (is_gimple_variable (t) && ! needs_to_live_in_memory (t)); } /* Return true if T is a GIMPLE rvalue, i.e. an identifier or a constant. */ diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c index 1ec24649204..a344384b426 100644 --- a/gcc/tree-ssa-alias.c +++ b/gcc/tree-ssa-alias.c @@ -390,7 +390,6 @@ init_alias_info (void) EXECUTE_IF_SET_IN_BITMAP (call_clobbered_vars, 0, i, { tree var = referenced_var (i); - DECL_NEEDS_TO_LIVE_IN_MEMORY_INTERNAL (var) = 0; /* Variables that are intrinsically call-clobbered (globals, local statics, etc) will not be marked by the aliasing @@ -1329,7 +1328,7 @@ setup_pointers_and_addressables (struct alias_info *ai) { if (!bitmap_bit_p (ai->addresses_needed, v_ann->uid) && v_ann->mem_tag_kind == NOT_A_TAG - && !needs_to_live_in_memory (var)) + && !is_global_var (var)) { /* The address of VAR is not needed, remove the addressable bit, so that it can be optimized as a @@ -1391,7 +1390,7 @@ setup_pointers_and_addressables (struct alias_info *ai) /* If pointer VAR is a global variable or a PARM_DECL, then its memory tag should be considered a global variable. */ - if (TREE_CODE (var) == PARM_DECL || needs_to_live_in_memory (var)) + if (TREE_CODE (var) == PARM_DECL || is_global_var (var)) mark_call_clobbered (tag); /* All the dereferences of pointer VAR count as @@ -2105,18 +2104,16 @@ get_nmt_for (tree ptr) tree tag = pi->name_mem_tag; if (tag == NULL_TREE) - { - tag = create_memory_tag (TREE_TYPE (TREE_TYPE (ptr)), false); + tag = create_memory_tag (TREE_TYPE (TREE_TYPE (ptr)), false); - /* If PTR is a PARM_DECL, its memory tag should be considered a - global variable. */ - if (TREE_CODE (SSA_NAME_VAR (ptr)) == PARM_DECL) - mark_call_clobbered (tag); + /* If PTR is a PARM_DECL, its memory tag should be considered a global + variable. */ + if (TREE_CODE (SSA_NAME_VAR (ptr)) == PARM_DECL) + mark_call_clobbered (tag); - /* Similarly, if PTR points to malloc, then TAG is a global. */ - if (pi->pt_malloc) - mark_call_clobbered (tag); - } + /* Similarly, if PTR points to malloc, then TAG is a global. */ + if (pi->pt_malloc) + mark_call_clobbered (tag); return tag; } diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c index c94a2f6c6d6..111f5400b9f 100644 --- a/gcc/tree-ssa-dce.c +++ b/gcc/tree-ssa-dce.c @@ -107,7 +107,6 @@ static inline basic_block find_pdom (basic_block); static inline void mark_stmt_necessary (tree, bool); static inline void mark_operand_necessary (tree); -static bool need_to_preserve_store (tree); static void mark_stmt_if_obviously_necessary (tree, bool); static void find_obviously_necessary_stmts (struct edge_list *); @@ -267,14 +266,6 @@ mark_operand_necessary (tree op) VARRAY_PUSH_TREE (worklist, stmt); } -/* Return true if a store to a variable needs to be preserved. */ - -static inline bool -need_to_preserve_store (tree ssa_name) -{ - return (needs_to_live_in_memory (SSA_NAME_VAR (ssa_name))); -} - /* Mark STMT as necessary if it is obviously is. Add it to the worklist if it can make other statements necessary. @@ -367,10 +358,11 @@ mark_stmt_if_obviously_necessary (tree stmt, bool aggressive) } ann = stmt_ann (stmt); - /* If the statement has volatile operands, it needs to be preserved. Same - for statements that can alter control flow in unpredictable ways. */ - if (ann->has_volatile_ops - || is_ctrl_altering_stmt (stmt)) + + /* If the statement has volatile operands, it needs to be preserved. + Same for statements that can alter control flow in unpredictable + ways. */ + if (ann->has_volatile_ops || is_ctrl_altering_stmt (stmt)) { mark_stmt_necessary (stmt, true); return; @@ -382,33 +374,87 @@ mark_stmt_if_obviously_necessary (tree stmt, bool aggressive) for (i = 0; i < NUM_DEFS (defs); i++) { tree def = DEF_OP (defs, i); - if (need_to_preserve_store (def)) + if (is_global_var (SSA_NAME_VAR (def))) { mark_stmt_necessary (stmt, true); return; } } + /* Check virtual definitions. If we get here, the only virtual + definitions we should see are those generated by assignment + statements. */ v_may_defs = V_MAY_DEF_OPS (ann); - for (i = 0; i < NUM_V_MAY_DEFS (v_may_defs); i++) - { - tree v_may_def = V_MAY_DEF_RESULT (v_may_defs, i); - if (need_to_preserve_store (v_may_def)) - { - mark_stmt_necessary (stmt, true); - return; - } - } - v_must_defs = V_MUST_DEF_OPS (ann); - for (i = 0; i < NUM_V_MUST_DEFS (v_must_defs); i++) + if (NUM_V_MAY_DEFS (v_may_defs) > 0 || NUM_V_MUST_DEFS (v_must_defs) > 0) { - tree v_must_def = V_MUST_DEF_OP (v_must_defs, i); - if (need_to_preserve_store (v_must_def)) + tree lhs; + +#if defined ENABLE_CHECKING + if (TREE_CODE (stmt) != MODIFY_EXPR) + abort (); +#endif + + /* Note that we must not check the individual virtual operands + here. In particular, if this is an aliased store, we could + end up with something like the following (SSA notation + redacted for brevity): + + foo (int *p, int i) + { + int x; + p_1 = (i_2 > 3) ? &x : p_1; + + # x_4 = V_MAY_DEF + *p_1 = 5; + + return 2; + } + + Notice that the store to '*p_1' should be preserved, if we + were to check the virtual definitions in that store, we would + not mark it needed. This is because 'x' is not a global + variable. + + Therefore, we check the base address of the LHS. If the + address is a pointer, we check if its name tag or type tag is + a global variable. Otherwise, we check if the base variable + is a global. */ + lhs = TREE_OPERAND (stmt, 0); + if (TREE_CODE_CLASS (TREE_CODE (lhs)) == 'r') + lhs = get_base_address (lhs); + + if (lhs == NULL_TREE) { + /* If LHS is NULL, it means that we couldn't get the base + address of the reference. In which case, we should not + remove this store. */ mark_stmt_necessary (stmt, true); - return; - } + } + else if (DECL_P (lhs)) + { + /* If the store is to a global symbol, we need to keep it. */ + if (is_global_var (lhs)) + mark_stmt_necessary (stmt, true); + } + else if (TREE_CODE (lhs) == INDIRECT_REF) + { + tree ptr = TREE_OPERAND (lhs, 0); + struct ptr_info_def *pi = SSA_NAME_PTR_INFO (ptr); + tree nmt = (pi) ? pi->name_mem_tag : NULL_TREE; + tree tmt = var_ann (SSA_NAME_VAR (ptr))->type_mem_tag; + + /* If either the name tag or the type tag for PTR is a + global variable, then the store is necessary. */ + if ((nmt && is_global_var (nmt)) + || (tmt && is_global_var (tmt))) + { + mark_stmt_necessary (stmt, true); + return; + } + } + else + abort (); } return; @@ -444,7 +490,7 @@ find_obviously_necessary_stmts (struct edge_list *el) Thus, we only need to mark PHIs for real variables which need their result preserved as being inherently necessary. */ if (is_gimple_reg (PHI_RESULT (phi)) - && need_to_preserve_store (PHI_RESULT (phi))) + && is_global_var (SSA_NAME_VAR (PHI_RESULT (phi)))) mark_stmt_necessary (phi, true); } diff --git a/gcc/tree-ssa-operands.c b/gcc/tree-ssa-operands.c index fe291d09d76..4b899eec526 100644 --- a/gcc/tree-ssa-operands.c +++ b/gcc/tree-ssa-operands.c @@ -1373,8 +1373,8 @@ get_call_expr_operands (tree stmt, tree expr) /* A 'pure' or a 'const' functions never call clobber anything. A 'noreturn' function might, but since we don't return anyway there is no point in recording that. */ - if (!(call_flags - & (ECF_PURE | ECF_CONST | ECF_NORETURN))) + if (TREE_SIDE_EFFECTS (expr) + && !(call_flags & (ECF_PURE | ECF_CONST | ECF_NORETURN))) add_call_clobber_ops (stmt); else if (!(call_flags & (ECF_CONST | ECF_NORETURN))) add_call_read_ops (stmt); diff --git a/gcc/tree.c b/gcc/tree.c index c8153b8f535..a6b17da494b 100644 --- a/gcc/tree.c +++ b/gcc/tree.c @@ -5624,15 +5624,22 @@ in_array_bounds_p (tree ref) return true; } +/* Return true if T (assumed to be a DECL) is a global variable. */ + +bool +is_global_var (tree t) +{ + return (TREE_STATIC (t) || DECL_EXTERNAL (t)); +} + /* Return true if T (assumed to be a DECL) must be assigned a memory location. */ bool needs_to_live_in_memory (tree t) { - return (DECL_NEEDS_TO_LIVE_IN_MEMORY_INTERNAL (t) - || TREE_STATIC (t) - || DECL_EXTERNAL (t) + return (TREE_ADDRESSABLE (t) + || is_global_var (t) || (TREE_CODE (t) == RESULT_DECL && aggregate_value_p (t, current_function_decl))); } diff --git a/gcc/tree.h b/gcc/tree.h index 0d6d5608310..e596f90d212 100644 --- a/gcc/tree.h +++ b/gcc/tree.h @@ -2161,15 +2161,6 @@ struct tree_binfo GTY (()) (! DECL_CONTEXT (EXP) \ || TREE_CODE (DECL_CONTEXT (EXP)) == TRANSLATION_UNIT_DECL) -/* Nonzero for a decl that has been marked as needing a memory slot. - NOTE: Never use this macro directly. It will give you incomplete - information. Most of the time this bit will only be set after alias - analysis in the tree optimizers. It's always better to call - needs_to_live_in_memory instead. To mark memory variables use - mark_call_clobbered. */ -#define DECL_NEEDS_TO_LIVE_IN_MEMORY_INTERNAL(DECL) \ - DECL_CHECK (DECL)->decl.needs_to_live_in_memory - /* Nonzero for a decl that cgraph has decided should be inlined into at least one call site. It is not meaningful to look at this directly; always use cgraph_function_possibly_inlined_p. */ @@ -2242,9 +2233,8 @@ struct tree_decl GTY(()) unsigned lang_flag_6 : 1; unsigned lang_flag_7 : 1; - unsigned needs_to_live_in_memory : 1; unsigned possibly_inlined : 1; - /* 14 unused bits. */ + /* 15 unused bits. */ union tree_decl_u1 { /* In a FUNCTION_DECL for which DECL_BUILT_IN holds, this is @@ -3479,6 +3469,7 @@ extern void expand_function_end (void); extern void expand_function_start (tree); extern void expand_pending_sizes (tree); extern void recompute_tree_invarant_for_addr_expr (tree); +extern bool is_global_var (tree t); extern bool needs_to_live_in_memory (tree); extern tree reconstruct_complex_type (tree, tree); diff --git a/libjava/ChangeLog b/libjava/ChangeLog index 1d2af7aba92..54e307cc844 100644 --- a/libjava/ChangeLog +++ b/libjava/ChangeLog @@ -1,3 +1,8 @@ +2004-08-12 Diego Novillo + + PR tree-optimization/16867 + * testsuite/libjava.lang/PR16867.java: New test. + 2004-08-09 Per Bothner * gcj/javaprims.h (_Jv_Utf8Const): Change struct to a class, diff --git a/libjava/testsuite/libjava.lang/PR16867.java b/libjava/testsuite/libjava.lang/PR16867.java new file mode 100644 index 00000000000..6862892304f --- /dev/null +++ b/libjava/testsuite/libjava.lang/PR16867.java @@ -0,0 +1,17 @@ +/* SSA-DCE was removing the initialization of the temporary object + in getFoo because it wasn't realizing that the pointer was needed + outside of it. */ + +public class PR16867 +{ + public static Object[] getFoo() + { + return new Object[] {"OK"}; + } + + public static void main(String[] args) + { + Object[] a = getFoo(); + System.out.println(a[0]); + } +} diff --git a/libjava/testsuite/libjava.lang/PR16867.out b/libjava/testsuite/libjava.lang/PR16867.out new file mode 100644 index 00000000000..d86bac9de59 --- /dev/null +++ b/libjava/testsuite/libjava.lang/PR16867.out @@ -0,0 +1 @@ +OK -- 2.30.2