Warnings fixes, i386 subdir

Zack Weinberg zack@wolery.cumb.org
Tue Feb 1 13:44:00 GMT 2000


Some uninitialized variables in i386.c, some string concatenation in
i386.h, and a tricky little bug in i386.h: the MACHINE_STATE_SAVE
macro was saving a callee-save register, which is unnecessary and
provokes uninitialized variable warnings.

	 * i386.c (ix86_expand_int_movcc): Add explicit 'return 0' in
	 all failure paths.
	 (ix86_flags_dependant): Likewise.  Disentangle control flow.
	 (ix86_sched_reorder): Break guts out to
	 ix86_sched_reorder_pentium and ix86_sched_reorder_ppro.
	 (ix86_sched_reorder_ppro): Initialize pair2 and insnp before
	 any possible use.

	 * i386.h (MACHINE_STATE_SAVE, MACHINE_STATE_RESTORE): Don't
	 use string concatenation.  Don't save and restore esi.

===================================================================
Index: config/i386/i386.c
--- config/i386/i386.c	2000/01/30 21:27:22	1.130
+++ config/i386/i386.c	2000/02/01 21:38:24
@@ -4917,7 +4917,7 @@ ix86_expand_int_movcc (operands)
     {
       /* Try a few things more with specific constants and a variable.  */
 
-      optab op = NULL;
+      optab op;
       rtx var, orig_out, out, tmp;
 
       if (optimize_size)
@@ -4933,6 +4933,8 @@ ix86_expand_int_movcc (operands)
 	    operands[3] = constm1_rtx, op = and_optab;
 	  else if (INTVAL (operands[2]) == -1)
 	    operands[3] = const0_rtx, op = ior_optab;
+	  else
+	    return 0; /* FAIL */
 	}
       else if (GET_CODE (operands[3]) == CONST_INT)
 	{
@@ -4941,9 +4943,10 @@ ix86_expand_int_movcc (operands)
 	    operands[2] = constm1_rtx, op = and_optab;
 	  else if (INTVAL (operands[3]) == -1)
 	    operands[2] = const0_rtx, op = ior_optab;
+	  else
+	    return 0; /* FAIL */
 	}
-
-      if (op == NULL)
+      else
         return 0; /* FAIL */
 
       orig_out = operands[0];
@@ -5901,17 +5904,21 @@ ix86_flags_dependant (insn, dep_insn, in
       set = SET_DEST (XVECEXP (PATTERN (dep_insn), 0, 0));
       set2 = SET_DEST (XVECEXP (PATTERN (dep_insn), 0, 0));
     }
+  else
+    return 0;
 
-  if (set && GET_CODE (set) == REG && REGNO (set) == FLAGS_REG)
-    {
-      /* This test is true if the dependant insn reads the flags but
-	 not any other potentially set register.  */
-      if (reg_overlap_mentioned_p (set, PATTERN (insn))
-	  && (!set2 || !reg_overlap_mentioned_p (set2, PATTERN (insn))))
-	return 1;
-    }
+  if (GET_CODE (set) != REG || REGNO (set) != FLAGS_REG)
+    return 0;
 
-  return 0;
+  /* This test is true if the dependant insn reads the flags but
+     not any other potentially set register.  */
+  if (!reg_overlap_mentioned_p (set, PATTERN (insn)))
+    return 0;
+
+  if (set2 && reg_overlap_mentioned_p (set2, PATTERN (insn)))
+    return 0;
+
+  return 1;
 }
 
 /* A subroutine of ix86_adjust_cost -- return true iff INSN has a memory
@@ -6208,166 +6215,185 @@ ix86_pent_find_pair (e_ready, ready, typ
   return bestinsnp;
 }
 
-/* We are about to being issuing insns for this clock cycle.  
-   Override the default sort algorithm to better slot instructions.  */
+/* Subroutines of ix86_sched_reorder.  */
 
-int
-ix86_sched_reorder (dump, sched_verbose, ready, n_ready, clock_var)
-     FILE *dump ATTRIBUTE_UNUSED;
-     int sched_verbose ATTRIBUTE_UNUSED;
+void
+ix86_sched_reorder_pentium (ready, e_ready)
      rtx *ready;
-     int n_ready, clock_var ATTRIBUTE_UNUSED;
+     rtx *e_ready;
 {
-  rtx *e_ready = ready + n_ready - 1;
+  enum attr_pent_pair pair1, pair2;
   rtx *insnp;
-  int i;
 
-  if (n_ready < 2)
-    goto out;
+  /* This wouldn't be necessary if Haifa knew that static insn ordering
+     is important to which pipe an insn is issued to.  So we have to make
+     some minor rearrangements.  */
 
-  switch (ix86_cpu)
+  pair1 = ix86_safe_pent_pair (*e_ready);
+
+  /* If the first insn is non-pairable, let it be.  */
+  if (pair1 == PENT_PAIR_NP)
+    return;
+
+  pair2 = PENT_PAIR_NP;
+  insnp = 0;
+
+  /* If the first insn is UV or PV pairable, search for a PU
+     insn to go with.  */
+  if (pair1 == PENT_PAIR_UV || pair1 == PENT_PAIR_PV)
     {
-    default:
-      goto out;
+      insnp = ix86_pent_find_pair (e_ready-1, ready,
+				   PENT_PAIR_PU, *e_ready);
+      if (insnp)
+	pair2 = PENT_PAIR_PU;
+    }
 
-    case PROCESSOR_PENTIUM:
-      /* This wouldn't be necessary if Haifa knew that static insn ordering
-	 is important to which pipe an insn is issued to.  So we have to make
-	 some minor rearrangements.  */
-      {
-	enum attr_pent_pair pair1, pair2;
+  /* If the first insn is PU or UV pairable, search for a PV
+     insn to go with.  */
+  if (pair2 == PENT_PAIR_NP
+      && (pair1 == PENT_PAIR_PU || pair1 == PENT_PAIR_UV))
+    {
+      insnp = ix86_pent_find_pair (e_ready-1, ready,
+				   PENT_PAIR_PV, *e_ready);
+      if (insnp)
+	pair2 = PENT_PAIR_PV;
+    }
 
-	pair1 = ix86_safe_pent_pair (*e_ready);
+  /* If the first insn is pairable, search for a UV
+     insn to go with.  */
+  if (pair2 == PENT_PAIR_NP)
+    {
+      insnp = ix86_pent_find_pair (e_ready-1, ready,
+				   PENT_PAIR_UV, *e_ready);
+      if (insnp)
+	pair2 = PENT_PAIR_UV;
+    }
 
-	/* If the first insn is non-pairable, let it be.  */
-	if (pair1 == PENT_PAIR_NP)
-	  goto out;
-	pair2 = PENT_PAIR_NP;
-
-	/* If the first insn is UV or PV pairable, search for a PU
-	   insn to go with.  */
-	if (pair1 == PENT_PAIR_UV || pair1 == PENT_PAIR_PV)
-	  {
-	    insnp = ix86_pent_find_pair (e_ready-1, ready,
-					 PENT_PAIR_PU, *e_ready);
-	    if (insnp)
-	      pair2 = PENT_PAIR_PU;
-	  }
+  if (pair2 == PENT_PAIR_NP)
+    return;
 
-	/* If the first insn is PU or UV pairable, search for a PV
-	   insn to go with.  */
-	if (pair2 == PENT_PAIR_NP
-	    && (pair1 == PENT_PAIR_PU || pair1 == PENT_PAIR_UV))
-	  {
-	    insnp = ix86_pent_find_pair (e_ready-1, ready,
-					 PENT_PAIR_PV, *e_ready);
-	    if (insnp)
-	      pair2 = PENT_PAIR_PV;
-	  }
+  /* Found something!  Decide if we need to swap the order.  */
+  if (pair1 == PENT_PAIR_PV || pair2 == PENT_PAIR_PU
+      || (pair1 == PENT_PAIR_UV && pair2 == PENT_PAIR_UV
+	  && ix86_safe_memory (*e_ready) == MEMORY_BOTH
+	  && ix86_safe_memory (*insnp) == MEMORY_LOAD))
+    ix86_reorder_insn (insnp, e_ready);
+  else
+    ix86_reorder_insn (insnp, e_ready - 1);
+}
 
-	/* If the first insn is pairable, search for a UV
-	   insn to go with.  */
-	if (pair2 == PENT_PAIR_NP)
-	  {
-	    insnp = ix86_pent_find_pair (e_ready-1, ready,
-					 PENT_PAIR_UV, *e_ready);
-	    if (insnp)
-	      pair2 = PENT_PAIR_UV;
-	  }
+void
+ix86_sched_reorder_ppro (ready, e_ready)
+     rtx *ready;
+     rtx *e_ready;
+{
+  rtx decode[3];
+  enum attr_ppro_uops cur_uops;
+  int issued_this_cycle;
+  rtx *insnp;
+  int i;
 
-	if (pair2 == PENT_PAIR_NP)
-	  goto out;
+  /* At this point .ppro.decode contains the state of the three 
+     decoders from last "cycle".  That is, those insns that were
+     actually independent.  But here we're scheduling for the 
+     decoder, and we may find things that are decodable in the
+     same cycle.  */
 
-	/* Found something!  Decide if we need to swap the order.  */
-	if (pair1 == PENT_PAIR_PV || pair2 == PENT_PAIR_PU
-	    || (pair1 == PENT_PAIR_UV && pair2 == PENT_PAIR_UV
-		&& ix86_safe_memory (*e_ready) == MEMORY_BOTH
-		&& ix86_safe_memory (*insnp) == MEMORY_LOAD))
-	  ix86_reorder_insn (insnp, e_ready);
-	else
-	  ix86_reorder_insn (insnp, e_ready - 1);
-      }
-      break;
+  memcpy (decode, ix86_sched_data.ppro.decode, sizeof(decode));
+  issued_this_cycle = 0;
 
-    case PROCESSOR_PENTIUMPRO:
-      {
-	rtx decode[3];
-	enum attr_ppro_uops cur_uops;
-	int issued_this_cycle;
-
-	/* At this point .ppro.decode contains the state of the three 
-	   decoders from last "cycle".  That is, those insns that were
-	   actually independant.  But here we're scheduling for the 
-	   decoder, and we may find things that are decodable in the
-	   same cycle.  */
+  insnp = e_ready;
+  cur_uops = ix86_safe_ppro_uops (*insnp);
+
+  /* If the decoders are empty, and we've a complex insn at the
+     head of the priority queue, let it issue without complaint.  */
+  if (decode[0] == NULL)
+    {
+      if (cur_uops == PPRO_UOPS_MANY)
+	{
+	  decode[0] = *insnp;
+	  goto ppro_done;
+	}
+
+      /* Otherwise, search for a 2-4 uop unsn to issue.  */
+      while (cur_uops != PPRO_UOPS_FEW)
+	{
+	  if (insnp == ready)
+	    break;
+	  cur_uops = ix86_safe_ppro_uops (*--insnp);
+	}
+
+      /* If so, move it to the head of the line.  */
+      if (cur_uops == PPRO_UOPS_FEW)
+	ix86_reorder_insn (insnp, e_ready);
 
-	memcpy (decode, ix86_sched_data.ppro.decode, sizeof(decode));
-	issued_this_cycle = 0;
+      /* Issue the head of the queue.  */
+      issued_this_cycle = 1;
+      decode[0] = *e_ready--;
+    }
 
+  /* Look for simple insns to fill in the other two slots.  */
+  for (i = 1; i < 3; ++i)
+    if (decode[i] == NULL)
+      {
+	if (ready >= e_ready)
+	  goto ppro_done;
+
 	insnp = e_ready;
 	cur_uops = ix86_safe_ppro_uops (*insnp);
-
-	/* If the decoders are empty, and we've a complex insn at the
-	   head of the priority queue, let it issue without complaint.  */
-	if (decode[0] == NULL)
+	while (cur_uops != PPRO_UOPS_ONE)
 	  {
-	    if (cur_uops == PPRO_UOPS_MANY)
-	      {
-		decode[0] = *insnp;
-		goto ppro_done;
-	      }
-
-	    /* Otherwise, search for a 2-4 uop unsn to issue.  */
-	    while (cur_uops != PPRO_UOPS_FEW)
-	      {
-		if (insnp == ready)
-		  break;
-		cur_uops = ix86_safe_ppro_uops (*--insnp);
-	      }
+	    if (insnp == ready)
+	      break;
+	    cur_uops = ix86_safe_ppro_uops (*--insnp);
+	  }
 
-	    /* If so, move it to the head of the line.  */
-	    if (cur_uops == PPRO_UOPS_FEW)
-	      ix86_reorder_insn (insnp, e_ready);
-
-	    /* Issue the head of the queue.  */
-	    issued_this_cycle = 1;
-	    decode[0] = *e_ready--;
+	/* Found one.  Move it to the head of the queue and issue it.  */
+	if (cur_uops == PPRO_UOPS_ONE)
+	  {
+	    ix86_reorder_insn (insnp, e_ready);
+	    decode[i] = *e_ready--;
+	    issued_this_cycle++;
+	    continue;
 	  }
 
-	/* Look for simple insns to fill in the other two slots.  */
-	for (i = 1; i < 3; ++i)
-	  if (decode[i] == NULL)
-	    {
-	      if (ready >= e_ready)
-		goto ppro_done;
+	/* ??? Didn't find one.  Ideally, here we would do a lazy split
+	   of 2-uop insns, issue one and queue the other.  */
+      }
 
-	      insnp = e_ready;
-	      cur_uops = ix86_safe_ppro_uops (*insnp);
-	      while (cur_uops != PPRO_UOPS_ONE)
-		{
-		  if (insnp == ready)
-		    break;
-		  cur_uops = ix86_safe_ppro_uops (*--insnp);
-		}
+ ppro_done:
+  if (issued_this_cycle == 0)
+    issued_this_cycle = 1;
+  ix86_sched_data.ppro.issued_this_cycle = issued_this_cycle;
+}
 
-	      /* Found one.  Move it to the head of the queue and issue it.  */
-	      if (cur_uops == PPRO_UOPS_ONE)
-		{
-		  ix86_reorder_insn (insnp, e_ready);
-		  decode[i] = *e_ready--;
-		  issued_this_cycle++;
-		  continue;
-		}
+  
+/* We are about to being issuing insns for this clock cycle.  
+   Override the default sort algorithm to better slot instructions.  */
+int
+ix86_sched_reorder (dump, sched_verbose, ready, n_ready, clock_var)
+     FILE *dump ATTRIBUTE_UNUSED;
+     int sched_verbose ATTRIBUTE_UNUSED;
+     rtx *ready;
+     int n_ready;
+     int clock_var ATTRIBUTE_UNUSED;
+{
+  rtx *e_ready = ready + n_ready - 1;
 
-	      /* ??? Didn't find one.  Ideally, here we would do a lazy split
-		 of 2-uop insns, issue one and queue the other.  */
-	    }
+  if (n_ready < 2)
+    goto out;
 
-      ppro_done:
-	if (issued_this_cycle == 0)
-	  issued_this_cycle = 1;
-	ix86_sched_data.ppro.issued_this_cycle = issued_this_cycle;
-      }
+  switch (ix86_cpu)
+    {
+    default:
+      break;
+
+    case PROCESSOR_PENTIUM:
+      ix86_sched_reorder_pentium (ready, e_ready);
+      break;
+
+    case PROCESSOR_PENTIUMPRO:
+      ix86_sched_reorder_ppro (ready, e_ready);
       break;
     }
 
===================================================================
Index: config/i386/i386.h
--- config/i386/i386.h	2000/01/25 05:59:17	1.96
+++ config/i386/i386.h	2000/02/01 21:38:26
@@ -1288,11 +1288,11 @@ typedef struct ix86_args {
 	Note that function `__bb_trace_ret' must not change the
 	machine state, especially the flag register. To grant
 	this, you must output code to save and restore registers
-	either in this macro or in the macros MACHINE_STATE_SAVE_RET
-	and MACHINE_STATE_RESTORE_RET. The last two macros will be
+	either in this macro or in the macros MACHINE_STATE_SAVE
+	and MACHINE_STATE_RESTORE. The last two macros will be
 	used in the function `__bb_trace_ret', so you must make
 	sure that the function prologue does not change any 
-	register prior to saving it with MACHINE_STATE_SAVE_RET.
+	register prior to saving it with MACHINE_STATE_SAVE.
 
    else if profiling_block_flag != 0:
 
@@ -1324,20 +1324,21 @@ emit_call_insn (gen_call (gen_rtx_MEM (P
    On the i386 the initialization code at the begin of
    function `__bb_trace_func' contains a `sub' instruction
    therefore we handle save and restore of the flag register 
-   in the BLOCK_PROFILER macro. */
+   in the BLOCK_PROFILER macro.
 
+   Note that ebx, esi, and edi are callee-save, so we don't have to
+   preserve them explicitly.  */
+
 #define MACHINE_STATE_SAVE(ID)					\
 do {								\
   register int eax_ __asm__("eax");				\
   register int ecx_ __asm__("ecx");				\
   register int edx_ __asm__("edx");				\
-  register int esi_ __asm__("esi");				\
-  __asm__ __volatile__ (					\
-	"push{l} %0\n\t"					\
-	"push{l} %1\n\t"					\
-	"push{l} %2\n\t"					\
-	"push{l} %3"						\
-	: : "r"(eax_), "r"(ecx_), "r"(edx_), "r"(esi_));	\
+  __asm__ __volatile__ ("\
+push{l} %0\n\t\
+push{l} %1\n\t\
+push{l} %2"							\
+	: : "r"(eax_), "r"(ecx_), "r"(edx_));			\
 } while (0);
 
 #define MACHINE_STATE_RESTORE(ID)				\
@@ -1345,13 +1346,11 @@ do {								\
   register int eax_ __asm__("eax");				\
   register int ecx_ __asm__("ecx");				\
   register int edx_ __asm__("edx");				\
-  register int esi_ __asm__("esi");				\
-  __asm__ __volatile__ (					\
-	"pop{l} %3\n\t"						\
-	"pop{l} %2\n\t"						\
-	"pop{l} %1\n\t"						\
-	"pop{l} %0"						\
-	: "=r"(eax_), "=r"(ecx_), "=r"(edx_), "=r"(esi_));	\
+  __asm__ __volatile__ ("\
+pop{l} %2\n\t\
+pop{l} %1\n\t\
+pop{l} %0"							\
+	: "=r"(eax_), "=r"(ecx_), "=r"(edx_));			\
 } while (0);
 
 /* EXIT_IGNORE_STACK should be nonzero if, when returning from a function,


More information about the Gcc-patches mailing list