Patchwork PATCH RFC: Patch for PR 47219

login
register
mail settings
Submitter Ian Taylor
Date Jan. 14, 2011, 12:49 a.m.
Message ID <mcrzkr4mhqo.fsf_-_@google.com>
Download mbox | patch
Permalink /patch/78855/
State New
Headers show

Comments

Ian Taylor - Jan. 14, 2011, 12:49 a.m.
Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> writes:

> Ian Lance Taylor <iant@google.com> writes:
>
>> I think I have now either committed or replied to each of these patches.
>
> I think so, yes.  Thanks for your amazing work of cleaning my hacks up.
>
>> Thanks for sending them.  I'm sure I got some things wrong, so give it
>> another try and see what happens.
>
> Will do, though that will probably have to wait for the weekend.  Any
> chance to have a look at PR go/47219?  Otherwise, this will have to
> remain x86-only for a while.

I'm pretty sure this is the correct patch, but I don't have a good way
to test it.  Would you be able to give it a try at some point?  I'll
preapprove this patch if it passes bootstrap and testing (without Go--Go
is irrelevant here).

What is happening here is that the SPARC backend calls new_alias_set
during initialization.  The new_alias_set function behaves differently
depending on whether flag_strict_aliasing is set or not.  The Go
frontend zeroes flag_strict_aliasing when it sees an import of the
"unsafe" package.  This then causes an assert which is double-checking
that if flag_strict_aliasing is not set, we are not using any alias
sets.

One way to handle this would be to add a
TARGET_OVERRIDE_OPTIONS_AFTER_CHANGE hook to sparc.c, which would let
the SPARC backend work correctly if some code uses __attribute__
((optimize ("no-strict-aliasing"))).

However, more generally, the SPARC backend is using two alias sets only
for access to the frame, and we already have gen_frame_mem for that
purpose.  We might as well do that, which has the advantage that
MEM_NOTRAP_P will also be set for the memory references.  This should
not cause any additional aliasing, because the frame references are easy
to disambiguate based only on the addresses.  Hence this patch.

Ian


2011-01-13  Ian Lance Taylor  <iant@google.com>

	* config/sparc/sparc.c (sparc_sr_alias_set): Don't define.
	(struct_value_alias_set): Don't define.
	(sparc_option_override): Don't set sparc_sr_alias_set and
	struct_value_alias_set.
	(save_or_restore_regs): Use gen_frame_mem rather than calling
	set_mem_alias_set.
	(sparc_struct_value_rtx): Likewise.
Eric Botcazou - Jan. 14, 2011, 8:34 a.m.
> I'm pretty sure this is the correct patch, but I don't have a good way
> to test it.  Would you be able to give it a try at some point?  I'll
> preapprove this patch if it passes bootstrap and testing (without Go--Go
> is irrelevant here).

I agree, this is fine for 4.6, thanks for debugging the problem.
Rainer Orth - Jan. 17, 2011, 1:47 p.m.
Eric Botcazou <ebotcazou@adacore.com> writes:

>> I'm pretty sure this is the correct patch, but I don't have a good way
>> to test it.  Would you be able to give it a try at some point?  I'll
>> preapprove this patch if it passes bootstrap and testing (without Go--Go
>> is irrelevant here).
>
> I agree, this is fine for 4.6, thanks for debugging the problem.

A sparc-sun-solaris2.11 bootstrap (with Go) finished without
regressions, so I've checked in the patch.

Thanks.
	Rainer

Patch

Index: gcc/config/sparc/sparc.c
===================================================================
--- gcc/config/sparc/sparc.c	(revision 168679)
+++ gcc/config/sparc/sparc.c	(working copy)
@@ -300,12 +300,6 @@  static HOST_WIDE_INT actual_fsize;
    saved (as 4-byte quantities).  */
 static int num_gfregs;
 
-/* The alias set for prologue/epilogue register save/restore.  */
-static GTY(()) alias_set_type sparc_sr_alias_set;
-
-/* The alias set for the structure return value.  */
-static GTY(()) alias_set_type struct_value_alias_set;
-
 /* Vector to say how input registers are mapped to output registers.
    HARD_FRAME_POINTER_REGNUM cannot be remapped by this function to
    eliminate it.  You must use -fomit-frame-pointer to get that.  */
@@ -912,10 +906,6 @@  sparc_option_override (void)
   /* Do various machine dependent initializations.  */
   sparc_init_modes ();
 
-  /* Acquire unique alias sets for our private stuff.  */
-  sparc_sr_alias_set = new_alias_set ();
-  struct_value_alias_set = new_alias_set ();
-
   /* Set up function hooks.  */
   init_machine_status = sparc_init_machine_status;
 
@@ -4381,8 +4371,7 @@  save_or_restore_regs (int low, int high,
 	{
 	  if (df_regs_ever_live_p (i) && ! call_used_regs[i])
 	    {
-	      mem = gen_rtx_MEM (DImode, plus_constant (base, offset));
-	      set_mem_alias_set (mem, sparc_sr_alias_set);
+	      mem = gen_frame_mem (DImode, plus_constant (base, offset));
 	      if (action == SORR_SAVE)
 		{
 		  insn = emit_move_insn (mem, gen_rtx_REG (DImode, i));
@@ -4422,8 +4411,7 @@  save_or_restore_regs (int low, int high,
 	  else
 	    continue;
 
-	  mem = gen_rtx_MEM (mode, plus_constant (base, offset));
-	  set_mem_alias_set (mem, sparc_sr_alias_set);
+	  mem = gen_frame_mem (mode, plus_constant (base, offset));
 	  if (action == SORR_SAVE)
 	    {
 	      insn = emit_move_insn (mem, gen_rtx_REG (mode, regno));
@@ -6087,11 +6075,11 @@  sparc_struct_value_rtx (tree fndecl, int
       rtx mem;
 
       if (incoming)
-	mem = gen_rtx_MEM (Pmode, plus_constant (frame_pointer_rtx,
-						 STRUCT_VALUE_OFFSET));
+	mem = gen_frame_mem (Pmode, plus_constant (frame_pointer_rtx,
+						   STRUCT_VALUE_OFFSET));
       else
-	mem = gen_rtx_MEM (Pmode, plus_constant (stack_pointer_rtx,
-						 STRUCT_VALUE_OFFSET));
+	mem = gen_frame_mem (Pmode, plus_constant (stack_pointer_rtx,
+						   STRUCT_VALUE_OFFSET));
 
       /* Only follow the SPARC ABI for fixed-size structure returns.
          Variable size structure returns are handled per the normal
@@ -6133,7 +6121,6 @@  sparc_struct_value_rtx (tree fndecl, int
 	  emit_label (endlab);
 	}
 
-      set_mem_alias_set (mem, struct_value_alias_set);
       return mem;
     }
 }