public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Patch for PR 33755: orphaned high-part relocations
@ 2007-10-22 22:28 Richard Sandiford
  2007-10-24 19:06 ` Richard Sandiford
  0 siblings, 1 reply; 2+ messages in thread
From: Richard Sandiford @ 2007-10-22 22:28 UTC (permalink / raw)
  To: gcc-patches; +Cc: echristo, daney

[-- Attachment #1: Type: text/plain, Size: 2589 bytes --]

PR 33755 is about a case in which 4.2 generates a %hi or local %got
relocation without a partnering %lo relocation.  This is wrong code,
and leads to a linker error.

The problem comes from dbr_schedule, although it's not really a bug there.
We have:

        bne     $5,$0,L1        # A
        ...stuff...
L1:
        bne     $5,$0,L2        # B
        ...printk call...
L2:

and nothing before dbr_schedule has managed to thread A to L2.
dbr_schedule first fills B's delay slot with an lui from the printk
block, then steal_delay_list_from_target realises that A can steal B's
delay slot and branch directly to L2.  There is no other path to L1,
so the rest of the printk call is now dead.

4.3 doesn't manage to thread the jump due to differences further up
the chain.  Both 4.2 and 4.3 versions expand the switch statement to
the following rtl:

   tmp = val & 1
   if (tmp == 0) goto ...
   tmp2 = 1
   if (tmp == tmp2) goto ...
   ...printk() code...

4.2 keeps the block in essentially this form up until combine,
which uses nonzero_bits checks to optimise the second branch into
"if (tmp != 0) goto ...".  4.3's loop-invariant motion can hoist
the setting of tmp2, thus stopping combine from doing the optimisation.

dbr_schedule isn't really where we'd want to optimise this case anyway.
The dbr_schedule is there for other situations, where the threading is
only possible when the delay slot is filled with an instruction from
the target of the branch.  We should ideally thread A to L2 much earlier,
preferably at the tree level, and delete the whole printk block as dead.
E.g. we probably ought to have some "nonzero bits" optimisations in tree
VRP, if we don't already.  (And if we don't already, I doubt I'll have been
the first person to say that.)

As far as 4.3 goes: if we need to keep the branch, what LIM is doing is
perfectly reasonable; if we don't need to keep the branch, we should have
optimised it away before LIM.

For most targets, this is at worst a missed optimisation.  I don't think
the MIPS port can rely on the optimisation for correctness.  So (alas!)
I think the upshot is simply that we need to add some special code to
mips_reorg to delete high-part relocations that have no matching lows.
This sort of bug is something that has happened off and on since at
least GCC 2 days, although the testcases tend to vary between releases,
so this particular example is a 4.2 regression.

Here are the patches I'm thinking of, one for mainline and 4.2.
I've tested both on mips-linux-gnu.  I'll hold off applying for
a couple of days for comments.

Richard



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: mainline.diff --]
[-- Type: text/x-diff, Size: 10740 bytes --]

gcc/
	PR target/33755
	* config/mips/mips.c (mips_lo_sum_offset): New structure.
	(mips_hash_base, mips_lo_sum_offset_hash, mips_lo_sum_offset_eq)
	(mips_lo_sum_offset_lookup, mips_record_lo_sum)
	(mips_orphaned_high_part_p: New functions.
	(mips_avoid_hazard): Don't check INSN_P here.
	(mips_avoid_hazards): Rename to...
	(mips_reorg_process_insns): ...this.  Cope with
	!TARGET_EXPLICIT_RELOCS.  Delete orphaned high-part relocations,
	or turn them into nops.
	(mips_reorg): Remove TARGET_EXPLICIT_RELOCS check from calls to
	dbr_schedule and mips_avoid_hazards/mips_reorg_process_insns.
	(mips_set_mips16_mode): Don't set flag_delayed_branch here.
	(mips_override_options): Set flag_delayed_branch to 0.

Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	2007-10-22 21:21:37.000000000 +0100
+++ gcc/config/mips/mips.c	2007-10-22 21:22:44.000000000 +0100
@@ -11184,8 +11184,122 @@ vr4130_align_insns (void)
   dfa_finish ();
 }
 \f
-/* Subroutine of mips_reorg.  If there is a hazard between INSN
-   and a previous instruction, avoid it by inserting nops after
+/* This structure records that the current function has a LO_SUM
+   involving SYMBOL_REF or LABEL_REF BASE and that MAX_OFFSET is
+   the largest offset applied to BASE by all such LO_SUMs.  */
+struct mips_lo_sum_offset {
+  rtx base;
+  HOST_WIDE_INT offset;
+};
+
+/* Return a hash value for SYMBOL_REF or LABEL_REF BASE.  */
+
+static hashval_t
+mips_hash_base (rtx base)
+{
+  int do_not_record_p;
+
+  return hash_rtx (base, GET_MODE (base), &do_not_record_p, NULL, false);
+}
+
+/* Hash-table callbacks for mips_lo_sum_offsets.  */
+
+static hashval_t
+mips_lo_sum_offset_hash (const void *entry)
+{
+  return mips_hash_base (((const struct mips_lo_sum_offset *) entry)->base);
+}
+
+static int
+mips_lo_sum_offset_eq (const void *entry, const void *value)
+{
+  return rtx_equal_p (((const struct mips_lo_sum_offset *) entry)->base,
+		      (const_rtx) value);
+}
+
+/* Look up symbolic constant X in HTAB, which is a hash table of
+   mips_lo_sum_offsets.  If OPTION is NO_INSERT, return true if X can be
+   paired with a recorded LO_SUM, otherwise record X in the table.  */
+
+static bool
+mips_lo_sum_offset_lookup (htab_t htab, rtx x, enum insert_option option)
+{
+  rtx base, offset;
+  void **slot;
+  struct mips_lo_sum_offset *entry;
+
+  /* Split X into a base and offset.  */
+  split_const (x, &base, &offset);
+  if (UNSPEC_ADDRESS_P (base))
+    base = UNSPEC_ADDRESS (base);
+
+  /* Look up the base in the hash table.  */
+  slot = htab_find_slot_with_hash (htab, base, mips_hash_base (base), option);
+  if (slot == NULL)
+    return false;
+
+  entry = (struct mips_lo_sum_offset *) *slot;
+  if (option == INSERT)
+    {
+      if (entry == NULL)
+	{
+	  entry = XNEW (struct mips_lo_sum_offset);
+	  entry->base = base;
+	  entry->offset = INTVAL (offset);
+	  *slot = entry;
+	}
+      else
+	{
+	  if (INTVAL (offset) > entry->offset)
+	    entry->offset = INTVAL (offset);
+	}
+    }
+  return INTVAL (offset) <= entry->offset;
+}
+
+/* A for_each_rtx callback for which DATA is a mips_lo_sum_offset hash table.
+   Record every LO_SUM in *LOC.  */
+
+static int
+mips_record_lo_sum (rtx *loc, void *data)
+{
+  if (GET_CODE (*loc) == LO_SUM)
+    mips_lo_sum_offset_lookup ((htab_t) data, XEXP (*loc, 1), INSERT);
+  return 0;
+}
+
+/* Return true if INSN is a SET of an orphaned high-part relocation.
+   HTAB is a hash table of mips_lo_sum_offsets that describes all the
+   LO_SUMs in the current function.  */
+
+static bool
+mips_orphaned_high_part_p (htab_t htab, rtx insn)
+{
+  enum mips_symbol_type type;
+  rtx x, set;
+
+  set = single_set (insn);
+  if (set)
+    {
+      /* Check for %his.  */
+      x = SET_SRC (set);
+      if (GET_CODE (x) == HIGH
+	  && absolute_symbolic_operand (XEXP (x, 0), VOIDmode))
+	return !mips_lo_sum_offset_lookup (htab, XEXP (x, 0), NO_INSERT);
+
+      /* Check for local %gots (and %got_pages, which is redundant but OK).  */
+      if (GET_CODE (x) == UNSPEC
+	  && XINT (x, 1) == UNSPEC_LOAD_GOT
+	  && mips_symbolic_constant_p (XVECEXP (x, 0, 1),
+				       SYMBOL_CONTEXT_LEA, &type)
+	  && type == SYMBOL_GOTOFF_PAGE)
+	return !mips_lo_sum_offset_lookup (htab, XVECEXP (x, 0, 1), NO_INSERT);
+    }
+  return false;
+}
+
+/* Subroutine of mips_reorg_process_insns.  If there is a hazard between
+   INSN and a previous instruction, avoid it by inserting nops after
    instruction AFTER.
 
    *DELAYED_REG and *HILO_DELAY describe the hazards that apply at
@@ -11206,9 +11320,6 @@ mips_avoid_hazard (rtx after, rtx insn, 
   rtx pattern, set;
   int nops, ninsns, hazard_set;
 
-  if (!INSN_P (insn))
-    return;
-
   pattern = PATTERN (insn);
 
   /* Do not put the whole function in .set noreorder if it contains
@@ -11268,14 +11379,16 @@ mips_avoid_hazard (rtx after, rtx insn, 
 }
 
 /* Go through the instruction stream and insert nops where necessary.
-   See if the whole function can then be put into .set noreorder &
-   .set nomacro.  */
+   Also delete any high-part relocations whose partnering low parts
+   are now all dead.  See if the whole function can then be put into
+   .set noreorder and .set nomacro.  */
 
 static void
-mips_avoid_hazards (void)
+mips_reorg_process_insns (void)
 {
-  rtx insn, last_insn, lo_reg, delayed_reg;
-  int hilo_delay, i;
+  rtx insn, last_insn, subinsn, next_insn, lo_reg, delayed_reg;
+  int hilo_delay;
+  htab_t htab;
 
   /* Force all instructions to be split into their final form.  */
   split_all_insns_noflow ();
@@ -11286,6 +11399,10 @@ mips_avoid_hazards (void)
 
   cfun->machine->all_noreorder_p = true;
 
+  /* Code that doesn't use explicit relocs can't be ".set nomacro".  */
+  if (!TARGET_EXPLICIT_RELOCS)
+    cfun->machine->all_noreorder_p = false;
+
   /* Profiled functions can't be all noreorder because the profiler
      support uses assembler macros.  */
   if (current_function_profile)
@@ -11303,24 +11420,63 @@ mips_avoid_hazards (void)
   if (TARGET_FIX_VR4130 && !ISA_HAS_MACCHI)
     cfun->machine->all_noreorder_p = false;
 
+  htab = htab_create (37, mips_lo_sum_offset_hash,
+		      mips_lo_sum_offset_eq, free);
+
+  /* Make a first pass over the instructions, recording all the LO_SUMs.  */
+  for (insn = get_insns (); insn != 0; insn = NEXT_INSN (insn))
+    FOR_EACH_SUBINSN (subinsn, insn)
+      if (INSN_P (subinsn))
+	for_each_rtx (&PATTERN (subinsn), mips_record_lo_sum, htab);
+
   last_insn = 0;
   hilo_delay = 2;
   delayed_reg = 0;
   lo_reg = gen_rtx_REG (SImode, LO_REGNUM);
 
-  for (insn = get_insns (); insn != 0; insn = NEXT_INSN (insn))
-    if (INSN_P (insn))
-      {
-	if (GET_CODE (PATTERN (insn)) == SEQUENCE)
-	  for (i = 0; i < XVECLEN (PATTERN (insn), 0); i++)
-	    mips_avoid_hazard (last_insn, XVECEXP (PATTERN (insn), 0, i),
-			       &hilo_delay, &delayed_reg, lo_reg);
-	else
-	  mips_avoid_hazard (last_insn, insn, &hilo_delay,
-			     &delayed_reg, lo_reg);
+  /* Make a second pass over the instructions.  Delete orphaned
+     high-part relocations or turn them into NOPs.  Avoid hazards
+     by inserting NOPs.  */
+  for (insn = get_insns (); insn != 0; insn = next_insn)
+    {
+      next_insn = NEXT_INSN (insn);
+      if (INSN_P (insn))
+	{
+	  if (GET_CODE (PATTERN (insn)) == SEQUENCE)
+	    {
+	      /* If we find an orphaned high-part relocation in a delay
+		 slot, it's easier to turn that instruction into a nop than
+		 to delete it.  The delay slot will be a nop either way.  */
+	      FOR_EACH_SUBINSN (subinsn, insn)
+		if (INSN_P (subinsn))
+		  {
+		    if (mips_orphaned_high_part_p (htab, subinsn))
+		      {
+			PATTERN (subinsn) = gen_nop ();
+			INSN_CODE (subinsn) = CODE_FOR_nop;
+		      }
+		    mips_avoid_hazard (last_insn, subinsn, &hilo_delay,
+				       &delayed_reg, lo_reg);
+		  }
+	      last_insn = insn;
+	    }
+	  else
+	    {
+	      /* INSN is a single instruction.  Delete it if it's an
+		 orphaned high-part relocation.  */
+	      if (mips_orphaned_high_part_p (htab, insn))
+		delete_insn (insn);
+	      else
+		{
+		  mips_avoid_hazard (last_insn, insn, &hilo_delay,
+				     &delayed_reg, lo_reg);
+		  last_insn = insn;
+		}
+	    }
+	}
+    }
 
-	last_insn = insn;
-      }
+  htab_delete (htab);
 }
 
 /* Implement TARGET_MACHINE_DEPENDENT_REORG.  */
@@ -11329,14 +11485,11 @@ mips_avoid_hazards (void)
 mips_reorg (void)
 {
   mips16_lay_out_constants ();
-  if (TARGET_EXPLICIT_RELOCS)
-    {
-      if (mips_base_delayed_branch)
-	dbr_schedule (get_insns ());
-      mips_avoid_hazards ();
-      if (TUNE_MIPS4130 && TARGET_VR4130_ALIGN)
-	vr4130_align_insns ();
-    }
+  if (mips_base_delayed_branch)
+    dbr_schedule (get_insns ());
+  mips_reorg_process_insns ();
+  if (TARGET_EXPLICIT_RELOCS && TUNE_MIPS4130 && TARGET_VR4130_ALIGN)
+    vr4130_align_insns ();
 }
 \f
 /* Implement TARGET_ASM_OUTPUT_MI_THUNK.  Generate rtl rather than asm text
@@ -11482,7 +11635,6 @@ mips_set_mips16_mode (int mips16_p)
 
   /* Restore base settings of various flags.  */
   target_flags = mips_base_target_flags;
-  flag_delayed_branch = mips_base_delayed_branch;
   flag_schedule_insns = mips_base_schedule_insns;
   flag_reorder_blocks_and_partition = mips_base_reorder_blocks_and_partition;
   flag_move_loop_invariants = mips_base_move_loop_invariants;
@@ -11535,11 +11687,6 @@ mips_set_mips16_mode (int mips16_p)
       /* Switch to normal (non-MIPS16) mode.  */
       target_flags &= ~MASK_MIPS16;
 
-      /* When using explicit relocs, we call dbr_schedule from within
-	 mips_reorg.  */
-      if (TARGET_EXPLICIT_RELOCS)
-	flag_delayed_branch = 0;
-
       /* Provide default values for align_* for 64-bit targets.  */
       if (TARGET_64BIT)
 	{
@@ -12064,6 +12211,9 @@ mips_override_options (void)
 
   /* Now select the ISA mode.  */
   mips_set_mips16_mode (mips_base_mips16);
+
+  /* We call dbr_schedule from within mips_reorg.  */
+  flag_delayed_branch = 0;
 }
 
 /* Swap the register information for registers I and I + 1, which
Index: gcc/testsuite/gcc.target/mips/pr33755.c
===================================================================
--- /dev/null	2007-10-17 09:55:09.552097000 +0100
+++ gcc/testsuite/gcc.target/mips/pr33755.c	2007-10-22 21:21:41.000000000 +0100
@@ -0,0 +1,30 @@
+/* { dg-do link } */
+/* { dg-mips-options "-O2" } */
+
+volatile int gv;
+const char *ptrs[2];
+
+void
+foo (volatile int *v, const char **ptrs)
+{
+  switch (*v & 1)
+    {
+    case 0:
+      ptrs[0] = 0;
+      break;
+    case 1:
+      break;
+    default:
+      ptrs[1] = "Some text";
+      break;
+    }
+  while (*v > 0)
+    *v -= 1;
+}
+
+int
+main (void)
+{
+  foo (&gv, ptrs);
+  return 0;
+}

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 4.2.diff --]
[-- Type: text/x-diff, Size: 10486 bytes --]

gcc/
	PR target/33755
	* config/mips/mips.c (override_options): Always move
	flag_delayed_branch to mips_flag_delayed_branch.
	(mips_lo_sum_offset): New structure.
	(mips_hash_base, mips_lo_sum_offset_hash, mips_lo_sum_offset_eq)
	(mips_lo_sum_offset_lookup, mips_record_lo_sum)
	(mips_orphaned_high_part_p: New functions.
	(mips_avoid_hazard): Don't check INSN_P here.
	(mips_avoid_hazards): Rename to...
	(mips_reorg_process_insns): ...this.  Cope with
	!TARGET_EXPLICIT_RELOCS.  Delete orphaned high-part relocations,
	or turn them into nops.
	(mips_reorg): Remove TARGET_EXPLICIT_RELOCS check from calls to
	dbr_schedule and mips_avoid_hazards/mips_reorg_process_insns.

gcc/testsuite/
	* gcc.target/mips/pr33755.c: New test.

Index: gcc/config/mips/mips.c
===================================================================
--- gcc/config/mips/mips.c	2007-10-22 21:15:28.000000000 +0100
+++ gcc/config/mips/mips.c	2007-10-22 21:22:39.000000000 +0100
@@ -361,7 +361,6 @@ static void mips_sim_finish_insn (struct
 static void vr4130_avoid_branch_rt_conflict (rtx);
 static void vr4130_align_insns (void);
 static void mips_avoid_hazard (rtx, rtx, int *, rtx *, rtx);
-static void mips_avoid_hazards (void);
 static void mips_reorg (void);
 static bool mips_strict_matching_cpu_name_p (const char *, const char *);
 static bool mips_matching_cpu_name_p (const char *, const char *);
@@ -4949,13 +4948,9 @@ override_options (void)
       target_flags &= ~MASK_EXPLICIT_RELOCS;
     }
 
-  /* When using explicit relocs, we call dbr_schedule from within
-     mips_reorg.  */
-  if (TARGET_EXPLICIT_RELOCS)
-    {
-      mips_flag_delayed_branch = flag_delayed_branch;
-      flag_delayed_branch = 0;
-    }
+  /* We call dbr_schedule from within mips_reorg.  */
+  mips_flag_delayed_branch = flag_delayed_branch;
+  flag_delayed_branch = 0;
 
 #ifdef MIPS_TFMODE_FORMAT
   REAL_MODE_FORMAT (TFmode) = &MIPS_TFMODE_FORMAT;
@@ -8973,8 +8968,122 @@ vr4130_align_insns (void)
   dfa_finish ();
 }
 \f
-/* Subroutine of mips_reorg.  If there is a hazard between INSN
-   and a previous instruction, avoid it by inserting nops after
+/* This structure records that the current function has a LO_SUM
+   involving SYMBOL_REF or LABEL_REF BASE and that MAX_OFFSET is
+   the largest offset applied to BASE by all such LO_SUMs.  */
+struct mips_lo_sum_offset {
+  rtx base;
+  HOST_WIDE_INT offset;
+};
+
+/* Return a hash value for SYMBOL_REF or LABEL_REF BASE.  */
+
+static hashval_t
+mips_hash_base (rtx base)
+{
+  int do_not_record_p;
+
+  return hash_rtx (base, GET_MODE (base), &do_not_record_p, NULL, false);
+}
+
+/* Hash-table callbacks for mips_lo_sum_offsets.  */
+
+static hashval_t
+mips_lo_sum_offset_hash (const void *entry)
+{
+  return mips_hash_base (((const struct mips_lo_sum_offset *) entry)->base);
+}
+
+static int
+mips_lo_sum_offset_eq (const void *entry, const void *value)
+{
+  return rtx_equal_p (((const struct mips_lo_sum_offset *) entry)->base,
+		      (rtx) value);
+}
+
+/* Look up symbolic constant X in HTAB, which is a hash table of
+   mips_lo_sum_offsets.  If OPTION is NO_INSERT, return true if X can be
+   paired with a recorded LO_SUM, otherwise record X in the table.  */
+
+static bool
+mips_lo_sum_offset_lookup (htab_t htab, rtx x, enum insert_option option)
+{
+  rtx base;
+  HOST_WIDE_INT offset;
+  void **slot;
+  struct mips_lo_sum_offset *entry;
+
+  /* Split X into a base and offset.  */
+  mips_split_const (x, &base, &offset);
+  if (UNSPEC_ADDRESS_P (base))
+    base = UNSPEC_ADDRESS (base);
+
+  /* Look up the base in the hash table.  */
+  slot = htab_find_slot_with_hash (htab, base, mips_hash_base (base), option);
+  if (slot == NULL)
+    return false;
+
+  entry = (struct mips_lo_sum_offset *) *slot;
+  if (option == INSERT)
+    {
+      if (entry == NULL)
+	{
+	  entry = XNEW (struct mips_lo_sum_offset);
+	  entry->base = base;
+	  entry->offset = offset;
+	  *slot = entry;
+	}
+      else
+	{
+	  if (offset > entry->offset)
+	    entry->offset = offset;
+	}
+    }
+  return offset <= entry->offset;
+}
+
+/* A for_each_rtx callback for which DATA is a mips_lo_sum_offset hash table.
+   Record every LO_SUM in *LOC.  */
+
+static int
+mips_record_lo_sum (rtx *loc, void *data)
+{
+  if (GET_CODE (*loc) == LO_SUM)
+    mips_lo_sum_offset_lookup ((htab_t) data, XEXP (*loc, 1), INSERT);
+  return 0;
+}
+
+/* Return true if INSN is a SET of an orphaned high-part relocation.
+   HTAB is a hash table of mips_lo_sum_offsets that describes all the
+   LO_SUMs in the current function.  */
+
+static bool
+mips_orphaned_high_part_p (htab_t htab, rtx insn)
+{
+  enum mips_symbol_type type;
+  rtx x, set;
+
+  set = single_set (insn);
+  if (set)
+    {
+      /* Check for %his.  */
+      x = SET_SRC (set);
+      if (GET_CODE (x) == HIGH
+	  && general_symbolic_operand (XEXP (x, 0), VOIDmode))
+	return !mips_lo_sum_offset_lookup (htab, XEXP (x, 0), NO_INSERT);
+
+      /* Check for local %gots (and %got_pages, which is redundant but OK).  */
+      if (GET_CODE (x) == UNSPEC
+	  && XINT (x, 1) == UNSPEC_LOAD_GOT
+	  && mips_symbolic_constant_p (XVECEXP (x, 0, 1), &type)
+	  && type == SYMBOL_GOTOFF_PAGE)
+	return !mips_lo_sum_offset_lookup (htab, XVECEXP (x, 0, 1), NO_INSERT);
+    }
+  return false;
+}
+
+/* Subroutine of mips_reorg_process_insns.  If there is a hazard between
+   INSN and a previous instruction, avoid it by inserting nops after
    instruction AFTER.
 
    *DELAYED_REG and *HILO_DELAY describe the hazards that apply at
@@ -8995,9 +9104,6 @@ mips_avoid_hazard (rtx after, rtx insn, 
   rtx pattern, set;
   int nops, ninsns, hazard_set;
 
-  if (!INSN_P (insn))
-    return;
-
   pattern = PATTERN (insn);
 
   /* Do not put the whole function in .set noreorder if it contains
@@ -9058,14 +9164,16 @@ mips_avoid_hazard (rtx after, rtx insn, 
 
 
 /* Go through the instruction stream and insert nops where necessary.
-   See if the whole function can then be put into .set noreorder &
-   .set nomacro.  */
+   Also delete any high-part relocations whose partnering low parts
+   are now all dead.  See if the whole function can then be put into
+   .set noreorder and .set nomacro.  */
 
 static void
-mips_avoid_hazards (void)
+mips_reorg_process_insns (void)
 {
-  rtx insn, last_insn, lo_reg, delayed_reg;
-  int hilo_delay, i;
+  rtx insn, last_insn, subinsn, next_insn, lo_reg, delayed_reg;
+  int hilo_delay;
+  htab_t htab;
 
   /* Force all instructions to be split into their final form.  */
   split_all_insns_noflow ();
@@ -9076,6 +9184,10 @@ mips_avoid_hazards (void)
 
   cfun->machine->all_noreorder_p = true;
 
+  /* Code that doesn't use explicit relocs can't be ".set nomacro".  */
+  if (!TARGET_EXPLICIT_RELOCS)
+    cfun->machine->all_noreorder_p = false;
+
   /* Profiled functions can't be all noreorder because the profiler
      support uses assembler macros.  */
   if (current_function_profile)
@@ -9093,24 +9205,63 @@ mips_avoid_hazards (void)
   if (TARGET_FIX_VR4130 && !ISA_HAS_MACCHI)
     cfun->machine->all_noreorder_p = false;
 
+  htab = htab_create (37, mips_lo_sum_offset_hash,
+		      mips_lo_sum_offset_eq, free);
+
+  /* Make a first pass over the instructions, recording all the LO_SUMs.  */
+  for (insn = get_insns (); insn != 0; insn = NEXT_INSN (insn))
+    FOR_EACH_SUBINSN (subinsn, insn)
+      if (INSN_P (subinsn))
+	for_each_rtx (&PATTERN (subinsn), mips_record_lo_sum, htab);
+
   last_insn = 0;
   hilo_delay = 2;
   delayed_reg = 0;
   lo_reg = gen_rtx_REG (SImode, LO_REGNUM);
 
-  for (insn = get_insns (); insn != 0; insn = NEXT_INSN (insn))
-    if (INSN_P (insn))
-      {
-	if (GET_CODE (PATTERN (insn)) == SEQUENCE)
-	  for (i = 0; i < XVECLEN (PATTERN (insn), 0); i++)
-	    mips_avoid_hazard (last_insn, XVECEXP (PATTERN (insn), 0, i),
-			       &hilo_delay, &delayed_reg, lo_reg);
-	else
-	  mips_avoid_hazard (last_insn, insn, &hilo_delay,
-			     &delayed_reg, lo_reg);
+  /* Make a second pass over the instructions.  Delete orphaned
+     high-part relocations or turn them into NOPs.  Avoid hazards
+     by inserting NOPs.  */
+  for (insn = get_insns (); insn != 0; insn = next_insn)
+    {
+      next_insn = NEXT_INSN (insn);
+      if (INSN_P (insn))
+	{
+	  if (GET_CODE (PATTERN (insn)) == SEQUENCE)
+	    {
+	      /* If we find an orphaned high-part relocation in a delay
+		 slot, it's easier to turn that instruction into a nop than
+		 to delete it.  The delay slot will be a nop either way.  */
+	      FOR_EACH_SUBINSN (subinsn, insn)
+		if (INSN_P (subinsn))
+		  {
+		    if (mips_orphaned_high_part_p (htab, subinsn))
+		      {
+			PATTERN (subinsn) = gen_nop ();
+			INSN_CODE (subinsn) = CODE_FOR_nop;
+		      }
+		    mips_avoid_hazard (last_insn, subinsn, &hilo_delay,
+				       &delayed_reg, lo_reg);
+		  }
+	      last_insn = insn;
+	    }
+	  else
+	    {
+	      /* INSN is a single instruction.  Delete it if it's an
+		 orphaned high-part relocation.  */
+	      if (mips_orphaned_high_part_p (htab, insn))
+		delete_insn (insn);
+	      else
+		{
+		  mips_avoid_hazard (last_insn, insn, &hilo_delay,
+				     &delayed_reg, lo_reg);
+		  last_insn = insn;
+		}
+	    }
+	}
+    }
 
-	last_insn = insn;
-      }
+  htab_delete (htab);
 }
 
 
@@ -9121,14 +9272,11 @@ mips_reorg (void)
 {
   if (TARGET_MIPS16)
     mips16_lay_out_constants ();
-  else if (TARGET_EXPLICIT_RELOCS)
-    {
-      if (mips_flag_delayed_branch)
-	dbr_schedule (get_insns ());
-      mips_avoid_hazards ();
-      if (TUNE_MIPS4130 && TARGET_VR4130_ALIGN)
-	vr4130_align_insns ();
-    }
+  if (mips_flag_delayed_branch)
+    dbr_schedule (get_insns ());
+  mips_reorg_process_insns ();
+  if (TARGET_EXPLICIT_RELOCS && TUNE_MIPS4130 && TARGET_VR4130_ALIGN)
+    vr4130_align_insns ();
 }
 
 /* This function does three things:
Index: gcc/testsuite/gcc.target/mips/pr33755.c
===================================================================
--- /dev/null	2007-10-17 09:55:09.552097000 +0100
+++ gcc/testsuite/gcc.target/mips/pr33755.c	2007-10-22 21:19:58.000000000 +0100
@@ -0,0 +1,30 @@
+/* { dg-do link } */
+/* { dg-mips-options "-O2" } */
+
+volatile int gv;
+const char *ptrs[2];
+
+void
+foo (volatile int *v, const char **ptrs)
+{
+  switch (*v & 1)
+    {
+    case 0:
+      ptrs[0] = 0;
+      break;
+    case 1:
+      break;
+    default:
+      ptrs[1] = "Some text";
+      break;
+    }
+  while (*v > 0)
+    *v -= 1;
+}
+
+int
+main (void)
+{
+  foo (&gv, ptrs);
+  return 0;
+}

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: Patch for PR 33755: orphaned high-part relocations
  2007-10-22 22:28 Patch for PR 33755: orphaned high-part relocations Richard Sandiford
@ 2007-10-24 19:06 ` Richard Sandiford
  0 siblings, 0 replies; 2+ messages in thread
From: Richard Sandiford @ 2007-10-24 19:06 UTC (permalink / raw)
  To: gcc-patches; +Cc: echristo, daney

Richard Sandiford <rsandifo@nildram.co.uk> writes:
> Here are the patches I'm thinking of, one for mainline and 4.2.
> I've tested both on mips-linux-gnu.  I'll hold off applying for
> a couple of days for comments.

No-one yelled in dismay, so I went ahead and applied them.

Richard

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2007-10-24 17:54 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-10-22 22:28 Patch for PR 33755: orphaned high-part relocations Richard Sandiford
2007-10-24 19:06 ` Richard Sandiford

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).