This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Fix PR14590, ppc-opc.c warning build breakage


PR14950 is regarding a compiler warning seen with gcc-3.4 cygwin.  I
suspect the same warning will be seen on any 32-bit gcc of that era,
and the code in question really does have a signed/unsigned comparison
when "long" is 32 bits.  It also depends on "int" being 32 bits.  So I
decided to tidy the function, and then noticed insert_sci8n and
extract_sci8n were quite bogus.

	PR binutils/14950
	* ppc-opc.c (insert_sci8, extract_sci8): Rewrite.
	(insert_sci8n, extract_sci8n): Likewise.

Index: opcodes/ppc-opc.c
===================================================================
RCS file: /cvs/src/src/opcodes/ppc-opc.c,v
retrieving revision 1.156
diff -u -p -r1.156 ppc-opc.c
--- opcodes/ppc-opc.c	26 Oct 2012 03:38:20 -0000	1.156
+++ opcodes/ppc-opc.c	13 Dec 2012 02:17:25 -0000
@@ -1634,62 +1634,50 @@ insert_sci8 (unsigned long insn,
 	     ppc_cpu_t dialect ATTRIBUTE_UNUSED,
 	     const char **errmsg)
 {
-  int fill = 0;
-  int scale_factor = 0;
-  long ui8 = value;
+  unsigned int fill_scale = 0;
+  unsigned long ui8 = value;
 
-  if ((value & 0xff000000) == (unsigned int) value)
+  if ((ui8 & 0xffffff00) == 0)
+    ;
+  else if ((ui8 & 0xffffff00) == 0xffffff00)
+    fill_scale = 0x400;
+  else if ((ui8 & 0xffff00ff) == 0)
     {
-      scale_factor = 3;
-      ui8 = value >> 24;
-      fill = 0;
+      fill_scale = 1 << 8;
+      ui8 >>= 8;
     }
-  else if ((value & 0xff0000) == (unsigned int) value)
+  else if ((ui8 & 0xffff00ff) == 0xffff00ff)
     {
-      scale_factor = 2;
-      ui8 = value >> 16;
-      fill = 0;
+      fill_scale = 0x400 | (1 << 8);
+      ui8 >>= 8;
     }
-  else if ((value & 0xff00) == (unsigned int) value)
+  else if ((ui8 & 0xff00ffff) == 0)
     {
-      scale_factor = 1;
-      ui8 = value >> 8;
-      fill = 0;
+      fill_scale = 2 << 8;
+      ui8 >>= 16;
     }
-  else if ((value & 0xff) == value)
+  else if ((ui8 & 0xff00ffff) == 0xff00ffff)
     {
-      scale_factor = 0;
-      ui8 = value;
-      fill = 0;
+      fill_scale = 0x400 | (2 << 8);
+      ui8 >>= 16;
     }
-  else if ((value & 0xffffff00) == 0xffffff00)
+  else if ((ui8 & 0x00ffffff) == 0)
     {
-      scale_factor = 0;
-      ui8 = (value & 0xff);
-      fill = 1;
+      fill_scale = 3 << 8;
+      ui8 >>= 24;
     }
-  else if ((value & 0xffff00ff) == 0xffff00ff)
+  else if ((ui8 & 0x00ffffff) == 0x00ffffff)
     {
-      scale_factor = 1;
-      ui8 = (value & 0xff00) >> 8;
-      fill = 1;
+      fill_scale = 0x400 | (3 << 8);
+      ui8 >>= 24;
     }
-  else if ((value & 0xff00ffff) == 0xff00ffff)
-    {
-      scale_factor = 2;
-      ui8 = (value & 0xff0000) >> 16;
-      fill = 1;
-    }
-  else if ((value & 0x00ffffff) == 0x00ffffff)
+  else
     {
-      scale_factor = 3;
-      ui8 = (value & 0xff000000) >> 24;
-      fill = 1;
+      *errmsg = _("illegal immediate value");
+      ui8 = 0;
     }
-  else
-    *errmsg = _("illegal immediate value");
 
-  return insn | (fill << 10) | (scale_factor << 8) | (ui8 & 0xff);
+  return insn | fill_scale | (ui8 & 0xff);
 }
 
 static long
@@ -1697,43 +1685,30 @@ extract_sci8 (unsigned long insn,
 	      ppc_cpu_t dialect ATTRIBUTE_UNUSED,
 	      int *invalid ATTRIBUTE_UNUSED)
 {
-  int scale_factor, fill;
-  scale_factor = (insn & 0x300) >> 8;
-  fill = (insn & 0x00000400) >> 10;
-
-  if (fill == 0)
-    return (insn & 0xff) << (scale_factor << 3);
-
-  /* Fill is one.  */
-  if (scale_factor == 0)
-    return (insn & 0xff) | 0xffffff00;
-  else if (scale_factor == 1)
-    return 0xffff00ff | ((insn & 0xff) << (scale_factor << 3));
-  else if (scale_factor == 2)
-    return 0xff00ffff | (insn & 0xff << (scale_factor << 3));
-  else /* scale_factor 3 */
-    return 0x00ffffff | (insn & 0xff << (scale_factor << 3));
+  int fill = insn & 0x400;
+  int scale_factor = (insn & 0x300) >> 5;
+  long value = (insn & 0xff) << scale_factor;
+
+  if (fill != 0)
+    value |= ~((long) 0xff << scale_factor);
+  return value;
 }
 
 static unsigned long
 insert_sci8n (unsigned long insn,
 	      long value,
-	      ppc_cpu_t dialect ATTRIBUTE_UNUSED,
+	      ppc_cpu_t dialect,
 	      const char **errmsg)
 {
-  insn = insert_sci8 (insn, -(value & 0xff) & 0xff, 0, errmsg);
-  /* Set the F bit.  */
-  return insn | 0x400;
+  return insert_sci8 (insn, -value, dialect, errmsg);
 }
 
 static long
 extract_sci8n (unsigned long insn,
-	       ppc_cpu_t dialect ATTRIBUTE_UNUSED,
-	       int *invalid ATTRIBUTE_UNUSED)
+	       ppc_cpu_t dialect,
+	       int *invalid)
 {
-  int scale_factor;
-  scale_factor = (insn & 0x300) >> 8;
-  return -(((insn & 0xff) ^ 0x80) - 0x80) << (scale_factor << 3);
+  return -extract_sci8 (insn, dialect, invalid);
 }
 
 static unsigned long

-- 
Alan Modra
Australia Development Lab, IBM


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]