-fstrict-aliasing fixes 4/6: do not fiddle with flag_strict_aliasing when expanding debug locations

-fstrict-aliasing fixes 4/6: do not fiddle with flag_strict_aliasing when expanding debug locations

[Jan Hubicka]

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?

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;
 }
 \f
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

Re: -fstrict-aliasing fixes 4/6: do not fiddle with flag_strict_aliasing when expanding debug locations

[Richard Biener]

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;
>  }
>  \f
> 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 <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)

Re: -fstrict-aliasing fixes 4/6: do not fiddle with flag_strict_aliasing when expanding debug locations

[Jakub Jelinek]

On Wed, Dec 02, 2015 at 10:05:13AM +0100, Richard Biener wrote:
> 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?

For aliasing purposes, like any other MEMs.  var-tracking needs to be able
to find out if some store through say some pointer could alias certain debug
MEM (then we need to flush the corresponding VALUEs), or not (then that
VALUE can be still considered live at that MEM location).

	Jakub

Re: -fstrict-aliasing fixes 4/6: do not fiddle with flag_strict_aliasing when expanding debug locations

[Richard Biener]

On Wed, 2 Dec 2015, Jakub Jelinek wrote:

> On Wed, Dec 02, 2015 at 10:05:13AM +0100, Richard Biener wrote:
> > 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?
> 
> For aliasing purposes, like any other MEMs.  var-tracking needs to be able
> to find out if some store through say some pointer could alias certain debug
> MEM (then we need to flush the corresponding VALUEs), or not (then that
> VALUE can be still considered live at that MEM location).

Ok, so we then pessimize the alias-set for all of them (when they
don't have a DECL_RTL already) to use alias-set zero even if we
wouldn't need to create a new alias set?  (and even if I don't see
what issues that would cause apart from "messing up the dumps" which
should be fixed by stripping alias sets from dumps)

Richard.

Re: -fstrict-aliasing fixes 4/6: do not fiddle with flag_strict_aliasing when expanding debug locations

[Jan Hubicka]

> On Wed, 2 Dec 2015, Jakub Jelinek wrote:
> 
> > On Wed, Dec 02, 2015 at 10:05:13AM +0100, Richard Biener wrote:
> > > 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?
> > 
> > For aliasing purposes, like any other MEMs.  var-tracking needs to be able
> > to find out if some store through say some pointer could alias certain debug
> > MEM (then we need to flush the corresponding VALUEs), or not (then that
> > VALUE can be still considered live at that MEM location).
> 
> Ok, so we then pessimize the alias-set for all of them (when they
> don't have a DECL_RTL already) to use alias-set zero even if we
> wouldn't need to create a new alias set?  (and even if I don't see
> what issues that would cause apart from "messing up the dumps" which
> should be fixed by stripping alias sets from dumps)

Breaking out code into make_decl_rtl_raw is also easy, since set_mem_attributes
is called at very end of make_decl_rtl.

Jakub, in https://gcc.gnu.org/ml/gcc-patches/2015-12/msg00316.html I make get_alias_set
to do the job even in !flas_strict_aliasing because this is needed to make it possible
to mix flag_strict_aliasing and !flag_strict_aliasing functions in one translation
unit.  We do that during LTO.

I wonder how to reproduce the messed up dumps here with that patch and if stripping
the alias sets would be OK? Indeed getting extra alias sets should not cause
codegen differences.

Honza
> 
> Richard.

Re: -fstrict-aliasing fixes 4/6: do not fiddle with flag_strict_aliasing when expanding debug locations

[Jan Hubicka]

Hi,
I discussed the situation with Jakub on IRC today.  It seems we still want
to have alias sets in the dumps, but Jakub agrees with introduction new global
var to control this instead of tampering with flag_strict_aliasing.

This patch does that and also fixes expand_call_stmt which forget to set the
flag.  Additionally I modified get_alias_set to lookup existing alias set
whever possible. In majority of cases the alias set is there because the
variable was optimized out and the optimizers thus touched it earlier.

Bootstrapped/regtested x86_64-linux including ada. OK?

Honza

	* cfgexpand.c: Include alias.h
	(expand_call_stmt, expand_debug_expr): Set no_new_alias_sets;
	do not fiddle with flag_strict_aliasing
	* alias.c (no_new_alias_sets): New global var.
	(get_alias_set): Do not introduce new alias sets when
	no_new_alias_sets is set.
	(new_alias_set): Check that no new alias sets are introduced.
	* alias.h (no_new_alias_sets): Declare.
	* varasm.c: Include alias.h
	(make_decl_rtl_for_debug): Use no_new_alias_sets instead of
	flag_strict_aliasing.
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
@@ -2622,12 +2623,17 @@ expand_call_stmt (gcall *stmt)
       unsigned int ix;
       tree dtemp;
 
+      gcc_assert (!no_new_alias_sets);
+      no_new_alias_sets = true;
+
       if (debug_args)
 	for (ix = 1; (*debug_args)->iterate (ix, &dtemp); ix += 2)
 	  {
 	    gcc_assert (TREE_CODE (dtemp) == DEBUG_EXPR_DECL);
 	    expand_debug_expr (dtemp);
 	  }
+
+      no_new_alias_sets = false;
     }
 
   lhs = gimple_call_lhs (stmt);
@@ -4080,6 +4086,8 @@ expand_debug_expr (tree exp)
   int unsignedp = TYPE_UNSIGNED (TREE_TYPE (exp));
   addr_space_t as;
 
+  gcc_assert (no_new_alias_sets);
+
   switch (TREE_CODE_CLASS (TREE_CODE (exp)))
     {
     case tcc_expression:
@@ -5219,12 +5227,12 @@ 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;
+  gcc_assert (!no_new_alias_sets);
+  no_new_alias_sets = true;
 
   for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
     if (DEBUG_INSN_P (insn))
@@ -5284,7 +5292,7 @@ expand_debug_locations (void)
 	  avoid_complex_debug_insns (insn2, &INSN_VAR_LOCATION_LOC (insn2), 0);
       }
 
-  flag_strict_aliasing = save_strict_alias;
+  no_new_alias_sets = false;
 }
 
 /* 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.  */
@@ -1487,7 +1488,6 @@ 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))
@@ -1496,18 +1496,16 @@ make_decl_rtl_for_debug (tree 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;
+     numbers off in the comparison dumps.  */
+  bool save_no_new_alias_sets = no_new_alias_sets;
+  no_new_alias_sets = 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);
 
-  flag_strict_aliasing = save_aliasing_flag;
+  no_new_alias_sets = save_no_new_alias_sets;
   return rtl;
 }
 \f
Index: alias.c
===================================================================
--- alias.c	(revision 231122)
+++ alias.c	(working copy)
@@ -145,6 +145,9 @@ struct GTY(()) alias_set_entry {
   bool has_pointer;
 };
 
+/* When true we will not allocate any new alias sets.  */
+bool no_new_alias_sets;
+
 static int rtx_equal_for_memref_p (const_rtx, const_rtx);
 static int memrefs_conflict_p (int, rtx, int, rtx, HOST_WIDE_INT);
 static void record_set (rtx, const_rtx, void *);
@@ -1034,6 +1037,10 @@ get_alias_set (tree t)
 	  gcc_checking_assert (p == TYPE_MAIN_VARIANT (p));
 	  if (TYPE_ALIAS_SET_KNOWN_P (p))
 	    set = TYPE_ALIAS_SET (p);
+	  /* During debug statement expanding we can not allocate new alias sets
+	     or the debug dumps will miscompare.  */
+	  else if (no_new_alias_sets)
+	    return 0;
 	  else
 	    {
 	      set = new_alias_set ();
@@ -1041,6 +1048,10 @@ get_alias_set (tree t)
 	    }
 	}
     }
+  /* During debug statement expanding we can not allocate new alias sets
+     or the debug dumps will miscompare.  */
+  else if (no_new_alias_sets)
+    return 0;
   /* Alias set of ptr_type_node is special and serve as universal pointer which
      is TBAA compatible with every other pointer type.  Be sure we have the
      alias set built even for LTO which otherwise keeps all TYPE_CANONICAL
@@ -1060,6 +1071,11 @@ get_alias_set (tree t)
       set = new_alias_set ();
     }
 
+  /* Do not record alias set 0 if we are not creating new aliases.  Perhaps
+     we needed new one during the recursion.  */
+  if (!set && !no_new_alias_sets)
+    return 0;
+
   TYPE_ALIAS_SET (t) = set;
 
   /* If this is an aggregate type or a complex type, we must record any
@@ -1085,6 +1101,7 @@ get_alias_set (tree t)
 alias_set_type
 new_alias_set (void)
 {
+  gcc_assert (!no_new_alias_sets);
   if (flag_strict_aliasing)
     {
       if (alias_sets == 0)
Index: alias.h
===================================================================
--- alias.h	(revision 231122)
+++ alias.h	(working copy)
@@ -37,6 +37,8 @@ extern void dump_alias_stats_in_alias_c
 tree reference_alias_ptr_type (tree);
 bool alias_ptr_types_compatible_p (tree, tree);
 
+extern bool no_new_alias_sets;
+
 /* This alias set can be used to force a memory to conflict with all
    other memories, creating a barrier across which no memory reference
    can move.  Note that there are other legacy ways to create such

Re: -fstrict-aliasing fixes 4/6: do not fiddle with flag_strict_aliasing when expanding debug locations

[Jakub Jelinek]

On Wed, Dec 02, 2015 at 09:16:10PM +0100, Jan Hubicka wrote:
> 	* cfgexpand.c: Include alias.h

Missing full stop at the end.

> 	(expand_call_stmt, expand_debug_expr): Set no_new_alias_sets;
> 	do not fiddle with flag_strict_aliasing

Likewise.

> 	* varasm.c: Include alias.h

Ditto.

> @@ -1034,6 +1037,10 @@ get_alias_set (tree t)
>  	  gcc_checking_assert (p == TYPE_MAIN_VARIANT (p));
>  	  if (TYPE_ALIAS_SET_KNOWN_P (p))
>  	    set = TYPE_ALIAS_SET (p);
> +	  /* During debug statement expanding we can not allocate new alias sets

s/expanding/expansion/ ?

> +  /* During debug statement expanding we can not allocate new alias sets

Ditto.

Otherwise, it looks reasonable to me, but please wait for richi's feedback
on it.

	Jakub

Re: -fstrict-aliasing fixes 4/6: do not fiddle with flag_strict_aliasing when expanding debug locations

[Richard Biener]

On Wed, 2 Dec 2015, Jakub Jelinek wrote:

> On Wed, Dec 02, 2015 at 09:16:10PM +0100, Jan Hubicka wrote:
> > 	* cfgexpand.c: Include alias.h
> 
> Missing full stop at the end.
> 
> > 	(expand_call_stmt, expand_debug_expr): Set no_new_alias_sets;
> > 	do not fiddle with flag_strict_aliasing
> 
> Likewise.
> 
> > 	* varasm.c: Include alias.h
> 
> Ditto.
> 
> > @@ -1034,6 +1037,10 @@ get_alias_set (tree t)
> >  	  gcc_checking_assert (p == TYPE_MAIN_VARIANT (p));
> >  	  if (TYPE_ALIAS_SET_KNOWN_P (p))
> >  	    set = TYPE_ALIAS_SET (p);
> > +	  /* During debug statement expanding we can not allocate new alias sets
> 
> s/expanding/expansion/ ?
> 
> > +  /* During debug statement expanding we can not allocate new alias sets
> 
> Ditto.
> 
> Otherwise, it looks reasonable to me, but please wait for richi's feedback
> on it.

Looks good to me.

Thanks,
Richard.