On Wed, 2 Dec 2015, Jan Hubicka wrote:
> Hi,
> this patch removes flag_strict_aliasing kludge in expanding debug locations
> and
> instead it introduces explicit parameter DEBUG that makes
> set_mem_attributes_minus_bitpos to not affect alias sets. This is sanity
> checked by comparing number of alias sets before and after at a time we
> originally overwritten flag_strict_aliasing.
>
> I also added code to prevent memory attributes creation for !optimize and to
> avoid get_alias_set computation for !flag_strict_aliasing. This slightly
> optimizes -O0 builds but the results seems to be down in the noise (I would
> not
> object to leave it out).
>
> The patch should fix at least one (latent?) bug that call_stmt expansion
> invoke expand_debug_expr without clearing flag_strict_aliasing.
>
> Bootstrapped/regtested x86_64-linux, also tested with compare-debug, OK?
First of all, why do debug MEMs need mem-attrs? Second, I'd rather
refactor make_decl_rtl into a _raw part that can be used from
both make_decl_rtl and make_decl_rtl_for_debug avoiding the debug
parameter.
I don't think any of this is suitable for stage3.
Thanks,
Richard.
> Honza
>
> * cfgexpand.c: Include alias.h
> (expand_debug_expr): Pass debug=true to set_mem_attributes.
> (expand_debug_locations): Do not fiddle with flag_strict_aliasing;
> sanity check that no new alias set was introduced.
> * varasm.c: Include alias.h
> (make_decl_rtl): New parameter DEBUG; pass it to set_mem_attributes.
> (make_decl_rtl_for_debug): Do ont fiddle with flag_strict_aliasing;
> assert that no new alias set was introduced.
> * varasm.h (make_decl_rtl): New parameter debug.
> * alias.h (num_alias_sets): New function.
> * emit-rtl.c (set_mem_attributes_minus_bitpos): New parameter DEBUG;
> exit early when not optimizing; do not introduce new alias set when
> producing debug only attributes.
> (set_mem_attributes): New parameter DEBUG.
> * emit-rtl.h (set_mem_attributes, set_mem_attributes_minus_bitpos):
> New parameters DEBUG.
> (num_alias_sets): New function.
>
> Index: cfgexpand.c
> ===================================================================
> --- cfgexpand.c (revision 231122)
> +++ cfgexpand.c (working copy)
> @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3.
> #include "builtins.h"
> #include "tree-chkp.h"
> #include "rtl-chkp.h"
> +#include "alias.h"
>
> /* Some systems use __main in a way incompatible with its use in gcc, in
> these
> cases use the macros NAME__MAIN to give a quoted symbol and SYMBOL__MAIN
> to
> @@ -4178,7 +4179,7 @@ expand_debug_expr (tree exp)
> return NULL_RTX;
> op0 = gen_rtx_CONST_STRING (Pmode, TREE_STRING_POINTER (exp));
> op0 = gen_rtx_MEM (BLKmode, op0);
> - set_mem_attributes (op0, exp, 0);
> + set_mem_attributes (op0, exp, 0, true);
> return op0;
> }
> /* Fall through... */
> @@ -4346,7 +4347,7 @@ expand_debug_expr (tree exp)
> return NULL;
>
> op0 = gen_rtx_MEM (mode, op0);
> - set_mem_attributes (op0, exp, 0);
> + set_mem_attributes (op0, exp, 0, true);
> if (TREE_CODE (exp) == MEM_REF
> && !is_gimple_mem_ref_addr (TREE_OPERAND (exp, 0)))
> set_mem_expr (op0, NULL_TREE);
> @@ -4372,7 +4373,7 @@ expand_debug_expr (tree exp)
>
> op0 = gen_rtx_MEM (mode, op0);
>
> - set_mem_attributes (op0, exp, 0);
> + set_mem_attributes (op0, exp, 0, true);
> set_mem_addr_space (op0, as);
>
> return op0;
> @@ -4458,7 +4459,7 @@ expand_debug_expr (tree exp)
> op0 = copy_rtx (op0);
> if (op0 == orig_op0)
> op0 = shallow_copy_rtx (op0);
> - set_mem_attributes (op0, exp, 0);
> + set_mem_attributes (op0, exp, 0, true);
> }
>
> if (bitpos == 0 && mode == GET_MODE (op0))
> @@ -5219,12 +5220,11 @@ expand_debug_locations (void)
> {
> rtx_insn *insn;
> rtx_insn *last = get_last_insn ();
> - int save_strict_alias = flag_strict_aliasing;
>
> /* New alias sets while setting up memory attributes cause
> -fcompare-debug failures, even though it doesn't bring about any
> codegen changes. */
> - flag_strict_aliasing = 0;
> + int num = num_alias_sets ();
>
> for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
> if (DEBUG_INSN_P (insn))
> @@ -5284,7 +5284,7 @@ expand_debug_locations (void)
> avoid_complex_debug_insns (insn2, &INSN_VAR_LOCATION_LOC (insn2), 0);
> }
>
> - flag_strict_aliasing = save_strict_alias;
> + gcc_checking_assert (num == num_alias_sets ());
> }
>
> /* Performs swapping operands of commutative operations to expand
> Index: varasm.c
> ===================================================================
> --- varasm.c (revision 231122)
> +++ varasm.c (working copy)
> @@ -52,6 +52,7 @@ along with GCC; see the file COPYING3.
> #include "common/common-target.h"
> #include "asan.h"
> #include "rtl-iter.h"
> +#include "alias.h"
>
> #ifdef XCOFF_DEBUGGING_INFO
> #include "xcoffout.h" /* Needed for external data
> declarations. */
> @@ -1280,7 +1281,7 @@ ultimate_transparent_alias_target (tree
> This is never called for PARM_DECL nodes. */
>
> void
> -make_decl_rtl (tree decl)
> +make_decl_rtl (tree decl, bool debug)
> {
> const char *name = 0;
> int reg_number;
> @@ -1470,7 +1471,7 @@ make_decl_rtl (tree decl)
>
> x = gen_rtx_MEM (DECL_MODE (decl), x);
> if (TREE_CODE (decl) != FUNCTION_DECL)
> - set_mem_attributes (x, decl, 1);
> + set_mem_attributes (x, decl, 1, debug);
> SET_DECL_RTL (decl, x);
>
> /* Optionally set flags or add text to the name to record information
> @@ -1487,27 +1488,25 @@ make_decl_rtl (tree decl)
> rtx
> make_decl_rtl_for_debug (tree decl)
> {
> - unsigned int save_aliasing_flag;
> rtx rtl;
>
> if (DECL_RTL_SET_P (decl))
> return DECL_RTL (decl);
>
> - /* Kludge alert! Somewhere down the call chain, make_decl_rtl will
> - call new_alias_set. If running with -fcompare-debug, sometimes
> - we do not want to create alias sets that will throw the alias
> - numbers off in the comparison dumps. So... clearing
> - flag_strict_aliasing will keep new_alias_set() from creating a
> - new set. */
> - save_aliasing_flag = flag_strict_aliasing;
> - flag_strict_aliasing = 0;
> + int num = num_alias_sets ();
>
> + make_decl_rtl (decl, true);
> rtl = DECL_RTL (decl);
> /* Reset DECL_RTL back, as various parts of the compiler expects
> DECL_RTL set meaning it is actually going to be output. */
> SET_DECL_RTL (decl, NULL);
> +
> + /* Be sure that make_decl_rtl will not cal lnew_alias set.
> + If running with -fcompare-debug, sometimes
> + we do not want to create alias sets that will throw the alias
> + numbers off in the comparison dumps. */
> + gcc_checking_assert (num == num_alias_sets ());
>
> - flag_strict_aliasing = save_aliasing_flag;
> return rtl;
> }
>
> Index: varasm.h
> ===================================================================
> --- varasm.h (revision 231122)
> +++ varasm.h (working copy)
> @@ -28,7 +28,7 @@ along with GCC; see the file COPYING3.
> extern tree cold_function_name;
>
> extern tree tree_output_constant_def (tree);
> -extern void make_decl_rtl (tree);
> +extern void make_decl_rtl (tree, bool debug = false);
> extern rtx make_decl_rtl_for_debug (tree);
> extern void make_decl_one_only (tree, tree);
> extern int supports_one_only (void);
> Index: alias.h
> ===================================================================
> --- alias.h (revision 231122)
> +++ alias.h (working copy)
> @@ -25,6 +25,7 @@ extern alias_set_type get_alias_set (tre
> extern alias_set_type get_deref_alias_set (tree);
> extern alias_set_type get_varargs_alias_set (void);
> extern alias_set_type get_frame_alias_set (void);
> +extern int num_alias_sets (void);
> extern tree component_uses_parent_alias_set_from (const_tree);
> extern bool alias_set_subset_of (alias_set_type, alias_set_type);
> extern void record_alias_subset (alias_set_type, alias_set_type);
> Index: emit-rtl.c
> ===================================================================
> --- emit-rtl.c (revision 231122)
> +++ emit-rtl.c (working copy)
> @@ -1738,16 +1738,20 @@ get_mem_align_offset (rtx mem, unsigned
> /* Given REF (a MEM) and T, either the type of X or the expression
> corresponding to REF, set the memory attributes. OBJECTP is nonzero
> if we are making a new object of this type. BITPOS is nonzero if
> - there is an offset outstanding on T that will be applied later. */
> + there is an offset outstanding on T that will be applied later.
> + if DEBUG is true, assume that the MEM will be used only for debug
> + insns. */
>
> void
> set_mem_attributes_minus_bitpos (rtx ref, tree t, int objectp,
> - HOST_WIDE_INT bitpos)
> + HOST_WIDE_INT bitpos, bool debug)
> {
> HOST_WIDE_INT apply_bitpos = 0;
> tree type;
> struct mem_attrs attrs, *defattrs, *refattrs;
> addr_space_t as;
> + bool global_var = false;
> + tree orig_t = t;
>
> /* It can happen that type_for_mode was given a mode for which there
> is no language-level type. In which case it returns NULL, which
> @@ -1759,18 +1763,8 @@ set_mem_attributes_minus_bitpos (rtx ref
> if (type == error_mark_node)
> return;
>
> - /* If we have already set DECL_RTL = ref, get_alias_set will get the
> - wrong answer, as it assumes that DECL_RTL already has the right alias
> - info. Callers should not set DECL_RTL until after the call to
> - set_mem_attributes. */
> - gcc_assert (!DECL_P (t) || ref != DECL_RTL_IF_SET (t));
> -
> memset (&attrs, 0, sizeof (attrs));
>
> - /* Get the alias set from the expression or type (perhaps using a
> - front-end routine) and use it. */
> - attrs.alias = get_alias_set (t);
> -
> MEM_VOLATILE_P (ref) |= TYPE_VOLATILE (type);
> MEM_POINTER (ref) = POINTER_TYPE_P (type);
>
> @@ -1829,6 +1823,12 @@ set_mem_attributes_minus_bitpos (rtx ref
> {
> tree base;
>
> + if (TREE_CODE (t) == CONST_DECL
> + || (TREE_CODE (t) == VAR_DECL
> + && auto_var_in_fn_p (t, current_function_decl))
> + || (TREE_CODE (t) == LABEL_DECL && DECL_NONLOCAL (t)))
> + global_var = true;
> +
> if (TREE_THIS_VOLATILE (t))
> MEM_VOLATILE_P (ref) = 1;
>
> @@ -1866,13 +1866,20 @@ set_mem_attributes_minus_bitpos (rtx ref
> as = TYPE_ADDR_SPACE (TREE_TYPE (base));
> }
>
> + /* When not optimizing, there is no need to attach memory attributes
> + unless we produce DECL_RTL of declaration that may later be used in a
> + function that is optimized. */
> + if (!optimize && !global_var)
> + return;
> +
> /* If this expression uses it's parent's alias set, mark it such
> that we won't change it. */
> - if (component_uses_parent_alias_set_from (t) != NULL_TREE)
> + if (!debug && (flag_strict_aliasing || global_var)
> + && component_uses_parent_alias_set_from (t) != NULL_TREE)
> MEM_KEEP_ALIAS_SET_P (ref) = 1;
>
> /* If this is a decl, set the attributes of the MEM from it. */
> - if (DECL_P (t))
> + else if (DECL_P (t))
> {
> attrs.expr = t;
> attrs.offset_known_p = true;
> @@ -1962,6 +1969,11 @@ set_mem_attributes_minus_bitpos (rtx ref
> obj_align = (obj_bitpos & -obj_bitpos);
> attrs.align = MAX (attrs.align, obj_align);
> }
> + /* When not optimizing, there is no need to attach memory attributes
> + unless we produce DECL_RTL of declaration that may later be used in a
> + function that is optimized. */
> + else if (!global_var && !optimize)
> + return;
>
> if (tree_fits_uhwi_p (new_size))
> {
> @@ -1980,15 +1992,29 @@ set_mem_attributes_minus_bitpos (rtx ref
> attrs.size += apply_bitpos / BITS_PER_UNIT;
> }
>
> + /* Get the alias set from the expression or type (perhaps using a
> + front-end routine) and use it. Never produce new alias sets when
> + the memory reference is used only for debug (so the alias numbers
> + are stable) and with !flag_strict_aliasing for all function local RTLs.
> */
> + if (!debug && (flag_strict_aliasing || global_var))
> + {
> + /* If we have already set DECL_RTL = ref, get_alias_set will get the
> + wrong answer, as it assumes that DECL_RTL already has the right alias
> + info. Callers should not set DECL_RTL until after the call to
> + set_mem_attributes. */
> + gcc_assert (!DECL_P (orig_t) || ref != DECL_RTL_IF_SET (orig_t));
> + attrs.alias = get_alias_set (orig_t);
> + }
> +
> /* Now set the attributes we computed above. */
> attrs.addrspace = as;
> set_mem_attrs (ref, &attrs);
> }
>
> void
> -set_mem_attributes (rtx ref, tree t, int objectp)
> +set_mem_attributes (rtx ref, tree t, int objectp, bool debug)
> {
> - set_mem_attributes_minus_bitpos (ref, t, objectp, 0);
> + set_mem_attributes_minus_bitpos (ref, t, objectp, 0, debug);
> }
>
> /* Set the alias set of MEM to SET. */
> Index: emit-rtl.h
> ===================================================================
> --- emit-rtl.h (revision 231122)
> +++ emit-rtl.h (working copy)
> @@ -484,13 +484,16 @@ extern rtx offset_address (rtx, rtx, uns
>
> /* Given REF, a MEM, and T, either the type of X or the expression
> corresponding to REF, set the memory attributes. OBJECTP is nonzero
> - if we are making a new object of this type. */
> -extern void set_mem_attributes (rtx, tree, int);
> + if we are making a new object of this type.
> + If DEBUG is true, the memory expression will be used only for debug
> + statements and in that case avoid creation of new alias set. */
> +extern void set_mem_attributes (rtx, tree, int, bool debug = false);
>
> /* Similar, except that BITPOS has not yet been applied to REF, so if
> we alter MEM_OFFSET according to T then we should subtract BITPOS
> expecting that it'll be added back in later. */
> -extern void set_mem_attributes_minus_bitpos (rtx, tree, int, HOST_WIDE_INT);
> +extern void set_mem_attributes_minus_bitpos (rtx, tree, int, HOST_WIDE_INT,
> + bool debug = false);
>
> /* Return OFFSET if XEXP (MEM, 0) - OFFSET is known to be ALIGN
> bits aligned for 0 <= OFFSET < ALIGN / BITS_PER_UNIT, or
> Index: alias.c
> ===================================================================
> --- alias.c (revision 231146)
> +++ alias.c (working copy)
> @@ -1096,6 +1096,17 @@ new_alias_set (void)
> return 0;
> }
>
> +/* Return number of alias sets; used for sanity checking that we did not
> + introduced new ones for debug statements. */
> +
> +int
> +num_alias_sets (void)
> +{
> + if (!alias_sets)
> + return 0;
> + return alias_sets->length ();
> +}
> +
> /* Indicate that things in SUBSET can alias things in SUPERSET, but that
> not everything that aliases SUPERSET also aliases SUBSET. For example,
> in C, a store to an `int' can alias a load of a structure containing an
>
>
--
Richard Biener <[email protected]>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
