[PATCH 1/2] ppc: extend opindex to 16 bits

[PATCH 1/2] ppc: extend opindex to 16 bits

[Dmitry Selyutin]

With the upcoming SVP64 extension[0] to PowerPC architecture, it became
evident that PowerPC operand indices no longer fit 8 bits. This patch
switches the underlying type to uint16_t, also introducing a special
typedef so that any future extension goes even smoother.

[0] https://libre-soc.org
---
 gas/config/tc-ppc.c  | 11 ++++++-----
 include/opcode/ppc.h |  5 ++++-
 opcodes/ppc-dis.c    | 12 ++++++------
 3 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/gas/config/tc-ppc.c b/gas/config/tc-ppc.c
index 72128af501..12a4314c94 100644
--- a/gas/config/tc-ppc.c
+++ b/gas/config/tc-ppc.c
@@ -1553,7 +1553,7 @@ ppc_target_format (void)
 static bool
 insn_validate (const struct powerpc_opcode *op)
 {
-  const unsigned char *o;
+  const ppc_opindex_t *o;
   uint64_t omask = op->mask;

   /* The mask had better not trim off opcode bits.  */
@@ -1634,8 +1634,9 @@ ppc_setup_opcodes (void)
       unsigned int i;

       /* An index into powerpc_operands is stored in struct fix
-        fx_pcrel_adjust which is 8 bits wide.  */
-      gas_assert (num_powerpc_operands < 256);
+        fx_pcrel_adjust which is 16 bits wide.
+         Also, this field is signed due to historical reasons.  */
+      gas_assert (num_powerpc_operands < PPC_OPINDEX_MAX);

       /* Check operand masks.  Code here and in the disassembler assumes
         all the 1's in the mask are contiguous.  */
@@ -3251,7 +3252,7 @@ md_assemble (char *str)
   char *s;
   const struct powerpc_opcode *opcode;
   uint64_t insn;
-  const unsigned char *opindex_ptr;
+  const ppc_opindex_t *opindex_ptr;
   int need_paren;
   int next_opindex;
   struct ppc_fixup fixups[MAX_INSN_FIXUPS];
@@ -3348,7 +3349,7 @@ md_assemble (char *str)
        {
          if (num_optional_operands == 0)
            {
-             const unsigned char *optr;
+             const ppc_opindex_t *optr;
              int total = 0;
              int provided = 0;
              int omitted;
diff --git a/include/opcode/ppc.h b/include/opcode/ppc.h
index a9c2529831..e4af2a9089 100644
--- a/include/opcode/ppc.h
+++ b/include/opcode/ppc.h
@@ -29,6 +29,9 @@ extern "C" {
 #endif

 typedef uint64_t ppc_cpu_t;
+typedef uint16_t ppc_opindex_t;
+
+#define PPC_OPINDEX_MAX INT16_MAX

 /* The opcode table is an array of struct powerpc_opcode.  */

@@ -60,7 +63,7 @@ struct powerpc_opcode
   /* An array of operand codes.  Each code is an index into the
      operand table.  They appear in the order which the operands must
      appear in assembly code, and are terminated by a zero.  */
-  unsigned char operands[8];
+  ppc_opindex_t operands[8];
 };

 /* The table itself is sorted by major opcode number, and is otherwise
diff --git a/opcodes/ppc-dis.c b/opcodes/ppc-dis.c
index 38ddeca262..45e8faeef5 100644
--- a/opcodes/ppc-dis.c
+++ b/opcodes/ppc-dis.c
@@ -546,7 +546,7 @@ operand_value_powerpc (const struct
powerpc_operand *operand,
 /* Determine whether the optional operand(s) should be printed.  */

 static bool
-skip_optional_operands (const unsigned char *opindex,
+skip_optional_operands (const ppc_opindex_t *opindex,
                        uint64_t insn, ppc_cpu_t dialect, bool *is_pcrel)
 {
   const struct powerpc_operand *operand;
@@ -592,7 +592,7 @@ lookup_powerpc (uint64_t insn, ppc_cpu_t dialect)
        opcode < opcode_end;
        ++opcode)
     {
-      const unsigned char *opindex;
+      const ppc_opindex_t *opindex;
       const struct powerpc_operand *operand;
       int invalid;

@@ -637,7 +637,7 @@ lookup_prefix (uint64_t insn, ppc_cpu_t dialect)
        opcode < opcode_end;
        ++opcode)
     {
-      const unsigned char *opindex;
+      const ppc_opindex_t *opindex;
       const struct powerpc_operand *operand;
       int invalid;

@@ -691,7 +691,7 @@ lookup_vle (uint64_t insn, ppc_cpu_t dialect)
       uint64_t table_mask = opcode->mask;
       bool table_op_is_short = PPC_OP_SE_VLE(table_mask);
       uint64_t insn2;
-      const unsigned char *opindex;
+      const ppc_opindex_t *opindex;
       const struct powerpc_operand *operand;
       int invalid;

@@ -746,7 +746,7 @@ lookup_spe2 (uint64_t insn, ppc_cpu_t dialect)
       uint64_t table_opcd = opcode->opcode;
       uint64_t table_mask = opcode->mask;
       uint64_t insn2;
-      const unsigned char *opindex;
+      const ppc_opindex_t *opindex;
       const struct powerpc_operand *operand;
       int invalid;

@@ -925,7 +925,7 @@ print_insn_powerpc (bfd_vma memaddr,

   if (opcode != NULL)
     {
-      const unsigned char *opindex;
+      const ppc_opindex_t *opindex;
       const struct powerpc_operand *operand;
       enum {
        need_comma = 0,
--
2.36.0

Re: [PATCH 1/2] ppc: extend opindex to 16 bits

[Alan Modra]

On Wed, May 11, 2022 at 08:59:19PM +0300, Dmitry Selyutin wrote:
> +         Also, this field is signed due to historical reasons.  */

Huh?  What makes you say that?

Also, your patch misses changing the following.  The existing 0xff
mask was from a time when fx_pcrel_adjust was a signed char.

      int opindex = fixP->fx_pcrel_adjust & 0xff;

This could be any of
      int opindex = fixP->fx_pcrel_adjust & 0xffff;
      int opindex = (uint16_t) fixP->fx_pcrel_adjust;
      uint16_t opindex = fixP->fx_pcrel_adjust;
Which you choose is a matter of style.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 1/2] ppc: extend opindex to 16 bits

[Dmitry Selyutin]

On Thu, May 12, 2022 at 3:13 AM Alan Modra <amodra@gmail.com> wrote:
> Huh?  What makes you say that?

That was my first impression, since the only negative value I found
was -1, and I initially thought that was just a placeholder value.
Thank you for noticing that! I changed the wording so that we only
stress that this is signed and (for now?) 16-bit.

> Also, your patch misses changing the following.  The existing 0xff
> mask was from a time when fx_pcrel_adjust was a signed char.
>
>       int opindex = fixP->fx_pcrel_adjust & 0xff;
>
> This could be any of
>       int opindex = fixP->fx_pcrel_adjust & 0xffff;
>       int opindex = (uint16_t) fixP->fx_pcrel_adjust;
>       uint16_t opindex = fixP->fx_pcrel_adjust;
> Which you choose is a matter of style.

Thank you! Since PPC code now has the typedef, I updated the patch to
use `(ppc_opindex_t)fixP->fx_pcrel_adjust` form.