[PATCH 0/2] extend opindex and fx_pcrel_adjust to 16 bits

[PATCH 0/2] extend opindex and fx_pcrel_adjust 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.
These patches extend both the operand index and the dependent
fx_pcrel_adjust field to 16 bits.
This issue has been discussed in binutils mailing list[1]; I've added
gdb-patches mailing list as well, per MAINTAINERS file instructions,
since I'm unsure if this can break the gdb interface.

You can also find these patches in perhaps a more convenient form in
the libresoc fork of binutils repository[2] (branch fx_pcrel_adjust).

[0] https://libre-soc.org
[1] https://sourceware.org/pipermail/binutils/2022-May/120775.html
[2] https://git.libre-soc.org/?p=binutils-gdb.git;a=log;h=refs/heads/fx_pcrel_adjust

Dmitry Selyutin (2):
  ppc: extend opindex to 16 bits
  gas/write: extend fx_pcrel_adjust to 16 bits

 gas/config/tc-ppc.c  | 11 ++++++-----
 gas/write.h          |  4 ++--
 include/opcode/ppc.h |  5 ++++-
 opcodes/ppc-dis.c    | 12 ++++++------
 4 files changed, 18 insertions(+), 14 deletions(-)

--
2.36.0

[PATCH v2 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  | 12 ++++++------
 include/opcode/ppc.h |  5 ++++-
 opcodes/ppc-dis.c    | 12 ++++++------
 3 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/gas/config/tc-ppc.c b/gas/config/tc-ppc.c
index 72128af501..9b29b6e15e 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,8 @@ 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 a 16-bit signed integer.  */
+      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 +3251,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 +3348,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;
@@ -7011,7 +7011,7 @@ md_apply_fix (fixS *fixP, valueT *valP, segT seg)
   if (fixP->fx_pcrel_adjust != 0)
     {
       /* This is a fixup on an instruction.  */
-      int opindex = fixP->fx_pcrel_adjust & 0xff;
+      ppc_opindex_t opindex = (ppc_opindex_t)fixP->fx_pcrel_adjust;
 
       operand = &powerpc_operands[opindex];
 #ifdef OBJ_XCOFF
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

[PATCH v2 2/2] gas/write: extend fx_pcrel_adjust to 16 bits

[Dmitry Selyutin]

PowerPC code stores operand index into fx_pcrel_adjust field of fix
struct. Once count of PowerPC operands exceeds an 8-bit integer, the
code won't be able to store operand index anymore.
This patch extends the aforementioned field to 16 bits, exactly like
the ppc_opindex_t type; the missing 8 bits are taken from the fx_unused
field.
---
 gas/write.h | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/gas/write.h b/gas/write.h
index 501bdd828f..5469dce445 100644
--- a/gas/write.h
+++ b/gas/write.h
@@ -52,6 +52,12 @@ struct fix
   /* These small fields are grouped together for compactness of
      this structure, and efficiency of access on some architectures.  */
 
+  /* pc-relative offset adjust (only used by some CPU specific code) */
+  int fx_pcrel_adjust : 16;
+
+  /* How many bytes are involved? */
+  unsigned fx_size : 8;
+
   /* Is this a pc-relative relocation?  */
   unsigned fx_pcrel : 1;
 
@@ -73,13 +79,7 @@ struct fix
   unsigned fx_tcbit2 : 1;
 
   /* Spare bits.  */
-  unsigned fx_unused : 10;
-
-  /* pc-relative offset adjust (only used by some CPU specific code) */
-  int fx_pcrel_adjust : 8;
-
-  /* How many bytes are involved? */
-  unsigned fx_size : 8;
+  unsigned fx_unused : 2;
 
   bfd_reloc_code_real_type fx_r_type;
 
-- 
2.36.0

Re: [PATCH 0/2] extend opindex and fx_pcrel_adjust to 16 bits

[Dmitry Selyutin]

On Wed, May 11, 2022 at 8:59 PM Dmitry Selyutin <ghostmansd@gmail.com> wrote:
> With the upcoming SVP64 extension[0] to PowerPC architecture, it
> became evident that PowerPC operand indices no longer fit 8 bits.

Hello all, this is a kind reminder on v2 patches. Thank you for your
suggestions and remarks on the first revision.
The new revision has been submitted correctly, as part of a thread[0][1][2].
Do you have more comments or suggestions on these patches? Can these
be submitted?

[0] https://sourceware.org/pipermail/binutils/2022-May/120783.html
[1] https://sourceware.org/pipermail/binutils/2022-May/120804.html
[2] https://sourceware.org/pipermail/binutils/2022-May/120803.html

Re: [PATCH 0/2] extend opindex and fx_pcrel_adjust to 16 bits

[Dmitry Selyutin]

On Tue, May 17, 2022, 14:20 Dmitry Selyutin <ghostmansd@gmail.com> wrote:

> On Wed, May 11, 2022 at 8:59 PM Dmitry Selyutin <ghostmansd@gmail.com>
> wrote:
> > With the upcoming SVP64 extension[0] to PowerPC architecture, it
> > became evident that PowerPC operand indices no longer fit 8 bits.
>
> Hello all, this is a kind reminder on v2 patches. Thank you for your
> suggestions and remarks on the first revision.
>

Hello all, just in case the previous message was missed, I'd like to ping
about the patches again. The lack of space becomes major obstacle on
extending PPC opcodes, and it'd be great to contribute these patches. Also,
perhaps we'll find more opcode fields missing, like with the BC field[0].


[0] https://sourceware.org/pipermail/binutils/2022-May/120982.html

Re: [PATCH 0/2] extend opindex and fx_pcrel_adjust to 16 bits

[Alan Modra]

On Wed, May 25, 2022 at 12:09:15AM +0300, Dmitry Selyutin wrote:
> Hello all, just in case the previous message was missed, I'd like to ping
> about the patches again.

Sorry, I haven't been very active over the last few weeks.  I'm about
to apply the ppc_opindex_t patches, squashed into one, and your isel
patch.  I've made some comment changes, plus gone back to masking
fx_pcrel_adjust, the reason being that if someone needed to reduce
fx_pcrel_adjust to, say, 12 bits to accomodate some new struct fix
field, they could do so with only a change to PPC_OPINDEX_MAX in the
ppc target code.  A cast to ppc_opindex_t wouldn't work in that
situation I believe, you'd get the 12-bit signed value being extended
to an int, then masked to 16 bits by the cast.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 0/2] extend opindex and fx_pcrel_adjust to 16 bits

[Dmitry Selyutin]

It seems GMail smashed the addresses for some reason, effectively
joining Luke with binutils into a single entity. Let me quote the
whole mail so that it's delivered to everyone.

On Wed, May 25, 2022 at 6:56 AM Dmitry Selyutin <ghostmansd@gmail.com> wrote:
>
> On Wed, May 25, 2022 at 5:41 AM Alan Modra <amodra@gmail.com> wrote:
> >
> > I'm about to apply the ppc_opindex_t patches, squashed into one
>
> Shouldn't I get write access to the repo to be able to push on my own
> after the review? I couldn't find the exact instructions about how to
> get it, though.
>
> > I've made some comment changes, plus gone back to masking
>
> Sure, fine with me, perfectly reasonable. Thank you!

Re: [PATCH 0/2] extend opindex and fx_pcrel_adjust to 16 bits

[Alan Modra]

On Wed, May 25, 2022 at 07:11:16AM +0300, Dmitry Selyutin wrote:
> It seems GMail smashed the addresses for some reason, effectively

No, that was me.  I did bounce a second email with the addresses fixed.

-- 
Alan Modra
Australia Development Lab, IBM

Re: [PATCH 0/2] extend opindex and fx_pcrel_adjust to 16 bits

[Alan Modra]

On Wed, May 25, 2022 at 07:11:16AM +0300, Dmitry Selyutin wrote:
> > Shouldn't I get write access to the repo to be able to push on my own
> > after the review? I couldn't find the exact instructions about how to
> > get it, though.

Oops, forgot to reply to this part.  After you annoy us with enough
patches, we'll get tired of applying them for you and arrange write
access.  ;-)

-- 
Alan Modra
Australia Development Lab, IBM