This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH]: Merge reduced memory usage for memory tags from improved-aliasing-branch
- From: Ian Lance Taylor <ian at airs dot com>
- Cc: GCC Patches <gcc-patches at gcc dot gnu dot org>, Mark Mitchell <mark at codesourcery dot com>
- Date: 05 Dec 2005 11:41:06 -0800
- Subject: Re: [PATCH]: Merge reduced memory usage for memory tags from improved-aliasing-branch
- Nto: Daniel Berlin <dberlin@dberlin.org>
- References: <1133643402.7343.5.camel@linux.site>
Daniel Berlin <dberlin@dberlin.org> writes:
> - if (!(TREE_STATIC (var) || DECL_EXTERNAL (var))
> - && (var_ann (var)->mem_tag_kind == NOT_A_TAG
> - || var_ann (var)->mem_tag_kind == STRUCT_FIELD)
> + if (!(is_global_var (var))
> + && (!MTAG_P (var) || TREE_CODE (var) == STRUCT_FIELD_TAG)
The extra parentheses around the call to is_global_var are slighly
confusing. Please remove them.
> -#define SSA_VAR_P(DECL) \
> - (TREE_CODE (DECL) == VAR_DECL \
> - || TREE_CODE (DECL) == PARM_DECL \
> - || TREE_CODE (DECL) == RESULT_DECL \
> - || (TREE_CODE (DECL) == SSA_NAME \
> - && (TREE_CODE (SSA_NAME_VAR (DECL)) == VAR_DECL \
> - || TREE_CODE (SSA_NAME_VAR (DECL)) == PARM_DECL \
> - || TREE_CODE (SSA_NAME_VAR (DECL)) == RESULT_DECL)))
> +#define SSA_VAR_P(DECL) \
> + (TREE_CODE (DECL) == VAR_DECL \
> + || TREE_CODE (DECL) == PARM_DECL \
> + || TREE_CODE (DECL) == RESULT_DECL \
> + || MTAG_P (DECL) \
> + || (TREE_CODE (DECL) == SSA_NAME \
> + && (TREE_CODE (SSA_NAME_VAR (DECL)) == VAR_DECL \
> + || TREE_CODE (SSA_NAME_VAR (DECL)) == PARM_DECL \
> + || TREE_CODE (SSA_NAME_VAR (DECL)) == RESULT_DECL \
> + || MTAG_P (SSA_NAME_VAR (DECL)))))
It's a little hard to be sure, but I think there is an extra tab on
the #define line. Please correct if so.
> @@ -271,6 +271,9 @@ is_gimple_reg (tree t)
> if (TREE_CODE (t) == SSA_NAME)
> t = SSA_NAME_VAR (t);
>
> + if (MTAG_P (t))
> + return false;
> +
> if (!is_gimple_variable (t))
> return false;
>
> @@ -305,11 +308,12 @@ is_gimple_reg (tree t)
> if (TREE_CODE (TREE_TYPE (t)) == COMPLEX_TYPE)
> return DECL_COMPLEX_GIMPLE_REG_P (t);
>
> +
> /* Some compiler temporaries are created to be used exclusively in
> virtual operands (currently memory tags and sub-variables).
> These variables should never be considered GIMPLE registers. */
> if (DECL_ARTIFICIAL (t) && (ann = var_ann (t)) != NULL)
> - return ann->mem_tag_kind == NOT_A_TAG;
> + return !MTAG_P (t);
Extraneous blank line above the comment. Please remove it.
And this change in general doesn't make sense. You already tested
MTAG_P (t) at the start of the function, so you know it is not false.
My guess is that you can remove this test entirely. Please check
this.
> @@ -1500,7 +1499,7 @@ may_alias_p (tree ptr, HOST_WIDE_INT mem
>
> m_ann = var_ann (mem);
>
> - gcc_assert (m_ann->mem_tag_kind == TYPE_TAG);
> + gcc_assert (TREE_CODE (mem) == TYPE_MEMORY_TAG);
Can't you just remove the m_ann variable entirely? If so, please do.
> @@ -1769,16 +1793,9 @@ get_nmt_for (tree ptr)
> {
> struct ptr_info_def *pi = get_ptr_info (ptr);
> tree tag = pi->name_mem_tag;
> -
> +
Adds undesired trailing whitespace. Please don't.
> if (tag == NULL_TREE)
> tag = create_memory_tag (TREE_TYPE (TREE_TYPE (ptr)), false);
> -
> - /* If PTR is a PARM_DECL, it points to a global variable or malloc,
> - then its name tag should be considered a global variable. */
> - if (TREE_CODE (SSA_NAME_VAR (ptr)) == PARM_DECL
> - || pi->pt_global_mem)
> - mark_call_clobbered (tag);
> -
> return tag;
> }
This patch isn't in the ChangeLog, and it doesn't seem to be
mechanical. Are you sure you meant to include it in this patch?
Please send it separately.
> Index: tree.def
> ===================================================================
> --- tree.def (revision 107947)
> +++ tree.def (working copy)
> @@ -357,6 +357,9 @@ DEFTREECODE (CONST_DECL, "const_decl", t
> DEFTREECODE (PARM_DECL, "parm_decl", tcc_declaration, 0)
> DEFTREECODE (TYPE_DECL, "type_decl", tcc_declaration, 0)
> DEFTREECODE (RESULT_DECL, "result_decl", tcc_declaration, 0)
> +DEFTREECODE (STRUCT_FIELD_TAG, "struct_field_tag", tcc_declaration, 0)
> +DEFTREECODE (NAME_MEMORY_TAG, "name_memory_tag", tcc_declaration, 0)
> +DEFTREECODE (TYPE_MEMORY_TAG, "type_memory_tag", tcc_declaration, 0)
Please add a comment here explaining what these are.
> Index: tree-dfa.c
> ===================================================================
> --- tree-dfa.c (revision 107947)
> +++ tree-dfa.c (working copy)
> @@ -638,11 +638,15 @@ add_referenced_var (tree var, struct wal
> *slot = (void *) var;
>
> referenced_var_insert (DECL_UID (var), var);
> -
> +
Adds undesired trailing whitespace. Please don't.
> @@ -454,9 +462,12 @@ print_node (FILE *file, const char *pref
>
> print_node_brief (file, "context", DECL_CONTEXT (node), indent + 4);
>
> - print_node_brief (file, "attributes",
> - DECL_ATTRIBUTES (node), indent + 4);
> -
> + if (CODE_CONTAINS_STRUCT (code, TS_DECL_COMMON))
> + {
> + print_node_brief (file, "attributes",
> + DECL_ATTRIBUTES (node), indent + 4);
> + print_node_brief (file, "initial", DECL_INITIAL (node), indent + 4);
> + }
> if (CODE_CONTAINS_STRUCT (code, TS_DECL_WRTL))
> {
> print_node_brief (file, "abstract_origin",
> @@ -467,7 +478,6 @@ print_node (FILE *file, const char *pref
> print_node (file, "arguments", DECL_ARGUMENT_FLD (node), indent + 4);
> print_node (file, "result", DECL_RESULT_FLD (node), indent + 4);
> }
> - print_node_brief (file, "initial", DECL_INITIAL (node), indent + 4);
>
> lang_hooks.print_decl (file, node, indent);
Is anybody going to complain about this move of the "initial" field to
earlier in the printout? (I don't care myself.)
> Index: tree-ssa-structalias.c
> ===================================================================
> --- tree-ssa-structalias.c (revision 107947)
> +++ tree-ssa-structalias.c (working copy)
> @@ -2036,7 +2036,7 @@ bitpos_of_field (const tree fdecl)
> || TREE_CODE (DECL_FIELD_BIT_OFFSET (fdecl)) != INTEGER_CST)
> return -1;
>
> - return (tree_low_cst (DECL_FIELD_OFFSET (fdecl), 1) * 8)
> + return (tree_low_cst (DECL_FIELD_OFFSET (fdecl), 1) * BITS_PER_UNIT)
> + tree_low_cst (DECL_FIELD_BIT_OFFSET (fdecl), 1);
> }
This patch is neither mechanical nor in the ChangeLog. Please send it
separately.
The patch is approved with those changes. Technically I can't approve
the patch to cp/ptree.c, but it is only one line, and I think counts
as obvious even without the rest of the patch.
Thanks.
:REVIEWMAIL:
Ian