Use operand->extract to provide defaults for optional PowerPC operands

Most optional operands to powerpc instructions use a default value of
zero, but there are a few exceptions.  Those have been handled by
PPC_OPERAND_OPTIONAL_VALUE and an entry in the powerpc_operands table
for the default value, smuggled in the shift field.  This patch
changes that to using the operand extract function to provide non-zero
defaults.

I've also moved the code determining whether optional operands are
provided or omitted, to the point the first optional operand is seen,
and allowed for the possibility of optional base register operands
in a future patch.

The patch does change the error you get on invalid assembly like

  ld 3,4

You'll now see "missing operand" rather than
"syntax error; end of line, expected `('".

gas/
	* config/tc-ppc.c (md_assemble): Delay counting of optional
	operands until one is encountered.  Allow for the possibility
	of optional base regs, ie. PPC_OPERAND_PARENS.  Call
	ppc_optional_operand_value with extra args.
include/
	* opcode/ppc.h (struct powerpc_operand): Correct "insert" comment.
	Mention use of "extract" function to provide default value.
	(PPC_OPERAND_OPTIONAL_VALUE): Delete.
	(ppc_optional_operand_value): Rewrite to use extract function.
opcodes/
	* ppc-dis.c (operand_value_powerpc): Init "invalid".
	(skip_optional_operands): Count optional operands, and update
	ppc_optional_operand_value call.
	* ppc-opc.c (extract_dxdn): Remove ATTRIBUTE_UNUSED from used arg.
	(extract_vlensi): Likewise.
	(extract_fxm): Return default value for missing optional operand.
	(extract_ls, extract_raq, extract_tbr): Likewise.
	(insert_sxl, extract_sxl): New functions.
	(insert_esync, extract_esync): Remove Power9 handling and simplify.
	(powerpc_operands <FXM4, TBR>): Delete PPC_OPERAND_OPTIONAL_VALUE
	flag and extra entry.
	(powerpc_operands <SXL>): Likewise, and use insert_sxl and
	extract_sxl.

gas/ChangeLog

@@ -1,3 +1,10 @@
+2018-08-21  Alan Modra  <amodra@gmail.com>
+
+	* config/tc-ppc.c (md_assemble): Delay counting of optional
+	operands until one is encountered.  Allow for the possibility
+	of optional base regs, ie. PPC_OPERAND_PARENS.  Call
+	ppc_optional_operand_value with extra args.
+
 2018-08-21  Alan Modra  <amodra@gmail.com>
 
 	* testsuite/gas/s12z/bit-manip-invalid.d: Correct regexps.

gas/config/tc-ppc.c

@@ -2745,7 +2745,6 @@ md_assemble (char *str)
   const struct powerpc_opcode *opcode;
   uint64_t insn;
   const unsigned char *opindex_ptr;
-  int skip_optional;
   int need_paren;
   int next_opindex;
   struct ppc_fixup fixups[MAX_INSN_FIXUPS];
@@ -2783,55 +2782,11 @@ md_assemble (char *str)
     ++str;
 
   /* PowerPC operands are just expressions.  The only real issue is
-     that a few operand types are optional.  All cases which might use
+     that a few operand types are optional.  If an instruction has
-     an optional operand separate the operands only with commas (in some
+     multiple optional operands and one is omitted, then all optional
-     cases parentheses are used, as in ``lwz 1,0(1)'' but such cases never
+     operands past the first omitted one must also be omitted.  */
-     have optional operands).  Most instructions with optional operands
+  int num_optional_operands = 0;
-     have only one.  Those that have more than one optional operand can
+  int num_optional_provided = 0;
-     take either all their operands or none.  So, before we start seriously
-     parsing the operands, we check to see if we have optional operands,
-     and if we do, we count the number of commas to see which operands
-     have been omitted.  */
-  skip_optional = 0;
-  for (opindex_ptr = opcode->operands; *opindex_ptr != 0; opindex_ptr++)
-    {
-      const struct powerpc_operand *operand;
-
-      operand = &powerpc_operands[*opindex_ptr];
-      if ((operand->flags & PPC_OPERAND_OPTIONAL) != 0
-	  && !((operand->flags & PPC_OPERAND_OPTIONAL32) != 0 && ppc_obj64))
-	{
-	  unsigned int opcount;
-	  unsigned int num_operands_expected;
-
-	  /* There is an optional operand.  Count the number of
-	     commas in the input line.  */
-	  if (*str == '\0')
-	    opcount = 0;
-	  else
-	    {
-	      opcount = 1;
-	      s = str;
-	      while ((s = strchr (s, ',')) != (char *) NULL)
-		{
-		  ++opcount;
-		  ++s;
-		}
-	    }
-
-	  /* Compute the number of expected operands.  */
-	  for (num_operands_expected = 0, i = 0; opcode->operands[i]; i ++)
-	    ++ num_operands_expected;
-
-	  /* If there are fewer operands in the line then are called
-	     for by the instruction, we want to skip the optional
-	     operands.  */
-	  if (opcount < num_operands_expected)
-	    skip_optional = 1;
-
-	  break;
-	}
-    }
 
   /* Gather the operands.  */
   need_paren = 0;
@@ -2855,26 +2810,66 @@ md_assemble (char *str)
       errmsg = NULL;
 
       /* If this is an optional operand, and we are skipping it, just
-	 insert a zero.  */
+	 insert the default value, usually a zero.  */
       if ((operand->flags & PPC_OPERAND_OPTIONAL) != 0
-	  && !((operand->flags & PPC_OPERAND_OPTIONAL32) != 0 && ppc_obj64)
+	  && !((operand->flags & PPC_OPERAND_OPTIONAL32) != 0 && ppc_obj64))
-	  && skip_optional)
 	{
-	  int64_t val = ppc_optional_operand_value (operand);
+	  if (num_optional_operands == 0)
-	  if (operand->insert)
 	    {
-	      insn = (*operand->insert) (insn, val, ppc_cpu, &errmsg);
+	      const unsigned char *optr;
-	      if (errmsg != (const char *) NULL)
+	      int total = 0;
-		as_bad ("%s", errmsg);
+	      int provided = 0;
-	    }
+	      int omitted;
-	  else if (operand->shift >= 0)
-	    insn |= (val & operand->bitm) << operand->shift;
-	  else
-	    insn |= (val & operand->bitm) >> -operand->shift;
 
-	  if ((operand->flags & PPC_OPERAND_NEXT) != 0)
+	      s = str;
-	    next_opindex = *opindex_ptr + 1;
+	      for (optr = opindex_ptr; *optr != 0; optr++)
-	  continue;
+		{
+		  const struct powerpc_operand *op;
+		  op = &powerpc_operands[*optr];
+
+		  ++total;
+
+		  if ((op->flags & PPC_OPERAND_OPTIONAL) != 0
+		      && !((op->flags & PPC_OPERAND_OPTIONAL32) != 0
+			   && ppc_obj64))
+		    ++num_optional_operands;
+
+		  if (s != NULL && *s != '\0')
+		    {
+		      ++provided;
+
+		      /* Look for the start of the next operand.  */
+		      if ((op->flags & PPC_OPERAND_PARENS) != 0)
+			s = strpbrk (s, "(,");
+		      else
+			s = strchr (s, ',');
+
+		      if (s != NULL)
+			++s;
+		    }
+		}
+	      omitted = total - provided;
+	      num_optional_provided = num_optional_operands - omitted;
+	    }
+	  if (--num_optional_provided < 0)
+	    {
+	      int64_t val = ppc_optional_operand_value (operand, insn, ppc_cpu,
+							num_optional_provided);
+	      if (operand->insert)
+		{
+		  insn = (*operand->insert) (insn, val, ppc_cpu, &errmsg);
+		  if (errmsg != (const char *) NULL)
+		    as_bad ("%s", errmsg);
+		}
+	      else if (operand->shift >= 0)
+		insn |= (val & operand->bitm) << operand->shift;
+	      else
+		insn |= (val & operand->bitm) >> -operand->shift;
+
+	      if ((operand->flags & PPC_OPERAND_NEXT) != 0)
+		next_opindex = *opindex_ptr + 1;
+	      continue;
+	    }
 	}
 
       /* Gather the operand.  */
@@ -3448,27 +3443,28 @@ md_assemble (char *str)
 	    }
 	}
       else if ((operand->flags & PPC_OPERAND_PARENS) != 0)
-	{
+	endc = '(';
-	  endc = '(';
-	  need_paren = 1;
-	}
       else
 	endc = ',';
 
       /* The call to expression should have advanced str past any
 	 whitespace.  */
-      if (*str != endc
+      if (*str == endc)
-	  && (endc != ',' || *str != '\0'))
 	{
-	  if (*str == '\0')
+	  ++str;
-	    as_bad (_("syntax error; end of line, expected `%c'"), endc);
+	  if (endc == '(')
-	  else
+	    need_paren = 1;
-	    as_bad (_("syntax error; found `%c', expected `%c'"), *str, endc);
+	}
+      else if (*str != '\0')
+	{
+	  as_bad (_("syntax error; found `%c', expected `%c'"), *str, endc);
+	  break;
+	}
+      else if (endc == ')')
+	{
+	  as_bad (_("syntax error; end of line, expected `%c'"), endc);
 	  break;
 	}
-
-      if (*str != '\0')
-	++str;
     }
 
   while (ISSPACE (*str))

include/ChangeLog

@@ -1,3 +1,10 @@
+2018-08-21  Alan Modra  <amodra@gmail.com>
+
+	* opcode/ppc.h (struct powerpc_operand): Correct "insert" comment.
+	Mention use of "extract" function to provide default value.
+	(PPC_OPERAND_OPTIONAL_VALUE): Delete.
+	(ppc_optional_operand_value): Rewrite to use extract function.
+
 2018-08-18  John Darrington  <john@darrington.wattle.id.au>
 
 	* opcode/s12z.h: New file.

include/opcode/ppc.h

@@ -280,11 +280,10 @@ struct powerpc_operand
 
      If this field is not NULL, then simply call it with the
      instruction and the operand value.  It will return the new value
-     of the instruction.  If the ERRMSG argument is not NULL, then if
+     of the instruction.  If the operand value is illegal, *ERRMSG
-     the operand value is illegal, *ERRMSG will be set to a warning
+     will be set to a warning string (the operand will be inserted in
-     string (the operand will be inserted in any case).  If the
+     any case).  If the operand value is legal, *ERRMSG will be
-     operand value is legal, *ERRMSG will be unchanged (most operands
+     unchanged (most operands can accept any value).  */
-     can accept any value).  */
   uint64_t (*insert)
     (uint64_t instruction, int64_t op, ppc_cpu_t dialect, const char **errmsg);
 
@@ -302,11 +301,18 @@ struct powerpc_operand
      is the result).
 
      If this field is not NULL, then simply call it with the
-     instruction value.  It will return the value of the operand.  If
+     instruction value.  It will return the value of the operand.
-     the INVALID argument is not NULL, *INVALID will be set to
+     *INVALID will be set to one by the extraction function if this
-     non-zero if this operand type can not actually be extracted from
+     operand type can not be extracted from this operand (i.e., the
-     this operand (i.e., the instruction does not match).  If the
+     instruction does not match).  If the operand is valid, *INVALID
-     operand is valid, *INVALID will not be changed.  */
+     will not be changed.  *INVALID will always be non-negative when
+     used to extract a field from an instruction.
+
+     The extraction function is also called by both the assembler and
+     disassembler if an operand is optional, in which case the
+     function should return the default value of the operand.
+     *INVALID is negative in this case, and is the negative count of
+     omitted optional operands up to and including this operand.  */
   int64_t (*extract) (uint64_t instruction, ppc_cpu_t dialect, int *invalid);
 
   /* One bit syntax flags.  */
@@ -421,11 +427,6 @@ extern const unsigned int num_powerpc_operands;
    out regardless of the PPC_OPERAND_OPTIONAL field.  */
 #define PPC_OPERAND_NEXT (0x100000)
 
-/* This flag is only used with PPC_OPERAND_OPTIONAL.  If this operand
-   is omitted, then the value it should use for the operand is stored
-   in the SHIFT field of the immediatly following operand field.  */
-#define PPC_OPERAND_OPTIONAL_VALUE (0x200000)
-
 /* This flag is only used with PPC_OPERAND_OPTIONAL.  The operand is
    only optional when generating 32-bit code.  */
 #define PPC_OPERAND_OPTIONAL32 (0x400000)
@@ -464,10 +465,13 @@ extern const int powerpc_num_macros;
 extern ppc_cpu_t ppc_parse_cpu (ppc_cpu_t, ppc_cpu_t *, const char *);
 
 static inline int64_t
-ppc_optional_operand_value (const struct powerpc_operand *operand)
+ppc_optional_operand_value (const struct powerpc_operand *operand,
+			    uint64_t insn,
+			    ppc_cpu_t dialect,
+			    int num_optional)
 {
-  if ((operand->flags & PPC_OPERAND_OPTIONAL_VALUE) != 0)
+  if (operand->extract)
-    return (operand+1)->shift;
+    return (*operand->extract) (insn, dialect, &num_optional);
   return 0;
 }
 

opcodes/ChangeLog

@@ -1,3 +1,19 @@
+2018-08-21  Alan Modra  <amodra@gmail.com>
+
+	* ppc-dis.c (operand_value_powerpc): Init "invalid".
+	(skip_optional_operands): Count optional operands, and update
+	ppc_optional_operand_value call.
+	* ppc-opc.c (extract_dxdn): Remove ATTRIBUTE_UNUSED from used arg.
+	(extract_vlensi): Likewise.
+	(extract_fxm): Return default value for missing optional operand.
+	(extract_ls, extract_raq, extract_tbr): Likewise.
+	(insert_sxl, extract_sxl): New functions.
+	(insert_esync, extract_esync): Remove Power9 handling and simplify.
+	(powerpc_operands <FXM4, TBR>): Delete PPC_OPERAND_OPTIONAL_VALUE
+	flag and extra entry.
+	(powerpc_operands <SXL>): Likewise, and use insert_sxl and
+	extract_sxl.
+
 2018-08-20  Alan Modra  <amodra@gmail.com>
 
 	* sh-opc.h (MASK): Simplify.

opcodes/ppc-dis.c

@@ -451,7 +451,7 @@ operand_value_powerpc (const struct powerpc_operand *operand,
 		       uint64_t insn, ppc_cpu_t dialect)
 {
   int64_t value;
-  int invalid;
+  int invalid = 0;
   /* Extract the value from the instruction.  */
   if (operand->extract)
     value = (*operand->extract) (insn, dialect, &invalid);
@@ -484,15 +484,22 @@ skip_optional_operands (const unsigned char *opindex,
 			uint64_t insn, ppc_cpu_t dialect)
 {
   const struct powerpc_operand *operand;
+  int num_optional;
 
-  for (; *opindex != 0; opindex++)
+  for (num_optional = 0; *opindex != 0; opindex++)
     {
       operand = &powerpc_operands[*opindex];
-      if ((operand->flags & PPC_OPERAND_NEXT) != 0
+      if ((operand->flags & PPC_OPERAND_NEXT) != 0)
-	  || ((operand->flags & PPC_OPERAND_OPTIONAL) != 0
-	      && operand_value_powerpc (operand, insn, dialect) !=
-		 ppc_optional_operand_value (operand)))
 	return 0;
+      if ((operand->flags & PPC_OPERAND_OPTIONAL) != 0)
+	{
+	  /* Negative count is used as a flag to extract function.  */
+	  --num_optional;
+	  if (operand_value_powerpc (operand, insn, dialect)
+	      != ppc_optional_operand_value (operand, insn, dialect,
+					     num_optional))
+	    return 0;
+	}
     }
 
   return 1;

opcodes/ppc-opc.c

@@ -483,7 +483,7 @@ insert_dxdn (uint64_t insn,
 static int64_t
 extract_dxdn (uint64_t insn,
 	      ppc_cpu_t dialect ATTRIBUTE_UNUSED,
-	      int *invalid ATTRIBUTE_UNUSED)
+	      int *invalid)
 {
   return -extract_dxd (insn, dialect, invalid);
 }
@@ -537,8 +537,12 @@ extract_fxm (uint64_t insn,
 	     ppc_cpu_t dialect ATTRIBUTE_UNUSED,
 	     int *invalid)
 {
-  int64_t mask = (insn >> 12) & 0xff;
+  /* Return a value of -1 for a missing optional operand, which is
+     used as a flag by insert_fxm.  */
+  if (*invalid < 0)
+    return -1;
 
+  int64_t mask = (insn >> 12) & 0xff;
   /* Is this a Power4 insn?  */
   if ((insn & (1 << 20)) != 0)
     {
@@ -611,8 +615,11 @@ extract_ls (uint64_t insn,
 	    ppc_cpu_t dialect,
 	    int *invalid)
 {
-  uint64_t lvalue = (insn >> 21) & 3;
+  /* Missing optional operands have a value of zero.  */
+  if (*invalid < 0)
+    return 0;
 
+  uint64_t lvalue = (insn >> 21) & 3;
   if (((insn >> 1) & 0x3ff) == 598)
     {
       uint64_t max_lvalue = (dialect & PPC_OPCODE_POWER4) ? 2 : 1;
@@ -629,21 +636,13 @@ extract_ls (uint64_t insn,
 static uint64_t
 insert_esync (uint64_t insn,
 	      int64_t value,
-	      ppc_cpu_t dialect,
+	      ppc_cpu_t dialect ATTRIBUTE_UNUSED,
 	      const char **errmsg)
 {
   uint64_t ls = (insn >> 21) & 0x03;
 
-  if (value == 0)
+  if (value != 0
-    {
+      && ((~value >> 1) & 0x1) != ls)
-      if (((dialect & PPC_OPCODE_E6500) != 0 && ls > 1)
-	  || ((dialect & PPC_OPCODE_POWER9) != 0 && ls > 2))
-	*errmsg = _("illegal L operand value");
-      return insn;
-    }
-
-  if ((ls & ~0x1)
-      || (((value >> 1) & 0x1) ^ ls) == 0)
     *errmsg = _("incompatible L operand value");
 
   return insn | ((value & 0xf) << 16);
@@ -651,23 +650,18 @@ insert_esync (uint64_t insn,
 
 static int64_t
 extract_esync (uint64_t insn,
-	       ppc_cpu_t dialect,
+	       ppc_cpu_t dialect ATTRIBUTE_UNUSED,
 	       int *invalid)
 {
+  if (*invalid < 0)
+    return 0;
+
   uint64_t ls = (insn >> 21) & 0x3;
-  uint64_t lvalue = (insn >> 16) & 0xf;
+  uint64_t value = (insn >> 16) & 0xf;
-
+  if (value != 0
-  if (lvalue == 0)
+      && ((~value >> 1) & 0x1) != ls)
-    {
-      if (((dialect & PPC_OPCODE_E6500) != 0 && ls > 1)
-	  || ((dialect & PPC_OPCODE_POWER9) != 0 && ls > 2))
-	*invalid = 1;
-    }
-  else if ((ls & ~0x1)
-	   || (((lvalue >> 1) & 0x1) ^ ls) == 0)
     *invalid = 1;
-
+  return value;
-  return lvalue;
 }
 
 /* The MB and ME fields in an M form instruction expressed as a single
@@ -914,9 +908,11 @@ extract_raq (uint64_t insn,
 	     ppc_cpu_t dialect ATTRIBUTE_UNUSED,
 	     int *invalid)
 {
+  if (*invalid < 0)
+    return 0;
+
   uint64_t rtvalue = (insn >> 21) & 0x1f;
   uint64_t ravalue = (insn >> 16) & 0x1f;
-
   if (ravalue == rtvalue)
     *invalid = 1;
   return ravalue;
@@ -1279,6 +1275,9 @@ extract_tbr (uint64_t insn,
 	     ppc_cpu_t dialect ATTRIBUTE_UNUSED,
 	     int *invalid)
 {
+  if (*invalid < 0)
+    return 268;
+
   int64_t ret = ((insn >> 16) & 0x1f) | ((insn >> 6) & 0x3e0);
   if (ret != 268 && ret != 269)
     *invalid = 1;
@@ -1461,7 +1460,7 @@ insert_vlensi (uint64_t insn,
 static int64_t
 extract_vlensi (uint64_t insn,
 		ppc_cpu_t dialect ATTRIBUTE_UNUSED,
-		int *invalid ATTRIBUTE_UNUSED)
+		int *invalid)
 {
   int64_t value = ((insn >> 10) & 0xf800) | (insn & 0x7ff);
   value = (value ^ 0x8000) - 0x8000;
@@ -1773,6 +1772,25 @@ extract_Ddd (uint64_t insn,
 {
   return ((insn >> 11) & 0x3) | ((insn << 2) & 0x4);
 }
+
+static uint64_t
+insert_sxl (uint64_t insn,
+	    int64_t value,
+	    ppc_cpu_t dialect ATTRIBUTE_UNUSED,
+	    const char **errmsg ATTRIBUTE_UNUSED)
+{
+  return insn | ((value & 0x1) << 11);
+}
+
+static int64_t
+extract_sxl (uint64_t insn,
+	     ppc_cpu_t dialect ATTRIBUTE_UNUSED,
+	     int *invalid)
+{
+  if (*invalid < 0)
+    return 1;
+  return (insn >> 11) & 0x1;
+}
 
 /* The operands table.
 
@@ -2071,13 +2089,10 @@ const struct powerpc_operand powerpc_operands[] =
 
   /* Power4 version for mfcr.  */
 #define FXM4 FXM + 1
-  { 0xff, 12, insert_fxm, extract_fxm,
+  { 0xff, 12, insert_fxm, extract_fxm, PPC_OPERAND_OPTIONAL },
-    PPC_OPERAND_OPTIONAL | PPC_OPERAND_OPTIONAL_VALUE},
-  /* If the FXM4 operand is ommitted, use the sentinel value -1.  */
-  { -1, -1, NULL, NULL, 0},
 
   /* The IMM20 field in an LI instruction.  */
-#define IMM20 FXM4 + 2
+#define IMM20 FXM4 + 1
   { 0xfffff, PPC_OPSHIFT_INV, insert_li20, extract_li20, PPC_OPERAND_SIGNED},
 
   /* The L field in a D or X form instruction.  */
@@ -2378,12 +2393,10 @@ const struct powerpc_operand powerpc_operands[] =
      field, but it is optional.  */
 #define TBR SV + 1
   { 0x3ff, 11, insert_tbr, extract_tbr,
-    PPC_OPERAND_SPR | PPC_OPERAND_OPTIONAL | PPC_OPERAND_OPTIONAL_VALUE},
+    PPC_OPERAND_SPR | PPC_OPERAND_OPTIONAL },
-  /* If the TBR operand is ommitted, use the value 268.  */
-  { -1, 268, NULL, NULL, 0},
 
   /* The TO field in a D or X form instruction.  */
-#define TO TBR + 2
+#define TO TBR + 1
 #define DUI TO
 #define TO_MASK (0x1f << 21)
   { 0x1f, 21, NULL, NULL, 0 },
@@ -2537,12 +2550,10 @@ const struct powerpc_operand powerpc_operands[] =
 
   /* The S field in a XL form instruction.  */
 #define SXL S + 1
-  { 0x1, 11, NULL, NULL, PPC_OPERAND_OPTIONAL | PPC_OPERAND_OPTIONAL_VALUE},
+  { 0x1, 11, insert_sxl, extract_sxl, PPC_OPERAND_OPTIONAL },
-  /* If the SXL operand is ommitted, use the value 1.  */
-  { -1, 1, NULL, NULL, 0},
 
   /* SH field starting at bit position 16.  */
-#define SH16 SXL + 2
+#define SH16 SXL + 1
   /* The DCM and DGM fields in a Z form instruction.  */
 #define DCM SH16
 #define DGM DCM