[PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions

[PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions

[Neal Frager]

This patches adds new bsefi and bsifi instructions.
BSEFI- The instruction shall extract a bit field from a
register and place it right-adjusted in the destination register.
The other bits in the destination register shall be set to zero.
BSIFI- The instruction shall insert a right-adjusted bit field
from a register at another position in the destination register.
The rest of the bits in the destination register shall be unchanged.

Further documentation of these instructions can be found here:
https://docs.xilinx.com/v/u/en-US/ug984-vivado-microblaze-ref

This patch has been tested for years of AMD Xilinx Yocto
releases as part of the following patch set:

https://github.com/Xilinx/meta-xilinx/tree/master/meta-microblaze/recipes-devtools/binutils/binutils

Signed-off-by: nagaraju <nagaraju.mekala@amd.com>
Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@amd.com>
Signed-off-by: Neal Frager <neal.frager@amd.com>
---
V1->V2:
 -corrected relocation values for the linker
---
 bfd/bfd-in2.h              |   5 ++
 bfd/elf32-microblaze.c     | 127 ++++++++++++++++++++++++++++---------
 bfd/libbfd.h               |   1 +
 bfd/reloc.c                |   6 ++
 binutils/readelf.c         |   4 ++
 gas/config/tc-microblaze.c |  77 +++++++++++++++++++++-
 include/elf/microblaze.h   |   1 +
 opcodes/microblaze-dis.c   |  16 +++++
 opcodes/microblaze-opc.h   |  12 +++-
 opcodes/microblaze-opcm.h  |   6 +-
 10 files changed, 223 insertions(+), 32 deletions(-)

diff --git a/bfd/bfd-in2.h b/bfd/bfd-in2.h
index eddb9902f5e..468e14af262 100644
--- a/bfd/bfd-in2.h
+++ b/bfd/bfd-in2.h
@@ -6461,6 +6461,11 @@ value relative to the read-write small data area anchor  */
 expressions of the form "Symbol Op Symbol"  */
   BFD_RELOC_MICROBLAZE_32_SYM_OP_SYM,
 
+/* This is a 32 bit reloc that stores the 32 bit pc relative
+value in two words (with an imm instruction).No relocation is 
+done here - only used for relaxing  */
+  BFD_RELOC_MICROBLAZE_32_NONE,
+
 /* This is a 64 bit reloc that stores the 32 bit pc relative
 value in two words (with an imm instruction).  No relocation is
 done here - only used for relaxing  */
diff --git a/bfd/elf32-microblaze.c b/bfd/elf32-microblaze.c
index a7e81c70fc8..1ce96ca6a88 100644
--- a/bfd/elf32-microblaze.c
+++ b/bfd/elf32-microblaze.c
@@ -174,6 +174,21 @@ static reloc_howto_type microblaze_elf_howto_raw[] =
 	  0x0000ffff,		/* Dest Mask.  */
 	  false),		/* PC relative offset?  */
 
+   /* This reloc does nothing.	Used for relaxation.  */
+   HOWTO (R_MICROBLAZE_32_NONE,	/* Type.  */
+          0,			/* Rightshift.  */
+          2,			/* Size (0 = byte, 1 = short, 2 = long).  */
+          32,			/* Bitsize.  */
+          true,			/* PC_relative.  */
+          0,			/* Bitpos.  */
+          complain_overflow_bitfield,  /* Complain on overflow.  */
+          NULL,                  /* Special Function.  */
+          "R_MICROBLAZE_32_NONE",/* Name.  */
+          false,		/* Partial Inplace.  */
+          0,			/* Source Mask.  */
+          0,			/* Dest Mask.  */
+          false),		/* PC relative offset?  */
+
    /* This reloc does nothing.	Used for relaxation.  */
    HOWTO (R_MICROBLAZE_64_NONE,	/* Type.  */
 	  0,			/* Rightshift.  */
@@ -560,6 +575,9 @@ microblaze_elf_reloc_type_lookup (bfd * abfd ATTRIBUTE_UNUSED,
     case BFD_RELOC_NONE:
       microblaze_reloc = R_MICROBLAZE_NONE;
       break;
+    case BFD_RELOC_MICROBLAZE_32_NONE:
+      microblaze_reloc = R_MICROBLAZE_32_NONE;
+      break;
     case BFD_RELOC_MICROBLAZE_64_NONE:
       microblaze_reloc = R_MICROBLAZE_64_NONE;
       break;
@@ -1954,18 +1972,26 @@ microblaze_elf_relax_section (bfd *abfd,
 		}
 	      break;
 	    case R_MICROBLAZE_NONE:
+	    case R_MICROBLAZE_32_NONE:
 	      {
 		/* This was a PC-relative instruction that was
 		   completely resolved.  */
 		size_t sfix, efix;
+		unsigned int val;
 		bfd_vma target_address;
 		target_address = irel->r_addend + irel->r_offset;
 		sfix = calc_fixup (irel->r_offset, 0, sec);
 		efix = calc_fixup (target_address, 0, sec);
-		irel->r_addend -= (efix - sfix);
-		/* Should use HOWTO.  */
-		microblaze_bfd_write_imm_value_32 (abfd, contents + irel->r_offset,
-						   irel->r_addend);
+
+                /* Validate the in-band val.  */
+                val = bfd_get_32 (abfd, contents + irel->r_offset);
+                if (val != irel->r_addend && ELF32_R_TYPE (irel->r_info) == R_MICROBLAZE_32_NONE) {
+                    fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, irel->r_addend);
+                }
+	        irel->r_addend -= (efix - sfix);
+	        /* Should use HOWTO.  */
+	        microblaze_bfd_write_imm_value_32 (abfd, contents + irel->r_offset,
+	                                           irel->r_addend);						   
 	      }
 	      break;
 	    case R_MICROBLAZE_64_NONE:
@@ -2009,31 +2035,74 @@ microblaze_elf_relax_section (bfd *abfd,
 	  irelscanend = irelocs + o->reloc_count;
 	  for (irelscan = irelocs; irelscan < irelscanend; irelscan++)
 	    {
-	      if (ELF32_R_TYPE (irelscan->r_info) == (int) R_MICROBLAZE_32)
-		{
-		  isym = isymbuf + ELF32_R_SYM (irelscan->r_info);
+              if (ELF32_R_TYPE (irelscan->r_info) == (int) R_MICROBLAZE_32_NONE)
+                {
+                  unsigned int val;
+
+                  isym = isymbuf + ELF32_R_SYM (irelscan->r_info);
+
+                  /* hax: We only do the following fixup for debug location lists.  */
+                  if (strcmp(".debug_loc", o->name))
+                    continue;
+
+                  /* This was a PC-relative instruction that was completely resolved.  */
+                  if (ocontents == NULL)
+                    {
+		      if (elf_section_data (o)->this_hdr.contents != NULL)
+		          ocontents = elf_section_data (o)->this_hdr.contents;
+		      else
+		        {
+		          /* We always cache the section contents.
+			     Perhaps, if info->keep_memory is FALSE, we
+			     should free them, if we are permitted to.  */
+
+		          if (o->rawsize == 0)
+			      o->rawsize = o->size;
+		          ocontents = (bfd_byte *) bfd_malloc (o->rawsize);
+		          if (ocontents == NULL)
+			      goto error_return;
+		          if (!bfd_get_section_contents (abfd, o, ocontents,
+                                                         (file_ptr) 0,
+                                                         o->rawsize))
+                              goto error_return;
+		          elf_section_data (o)->this_hdr.contents = ocontents;
+		        }
+		    }
 
-		  /* Look at the reloc only if the value has been resolved.  */
-		  if (isym->st_shndx == shndx
-		      && (ELF32_ST_TYPE (isym->st_info) == STT_SECTION))
-		    {
-		      if (ocontents == NULL)
-			{
-			  if (elf_section_data (o)->this_hdr.contents != NULL)
-			    ocontents = elf_section_data (o)->this_hdr.contents;
-			  else
-			    {
-			      /* We always cache the section contents.
-				 Perhaps, if info->keep_memory is FALSE, we
-				 should free them, if we are permitted to.  */
-			      if (o->rawsize == 0)
-				o->rawsize = o->size;
-			      ocontents = (bfd_byte *) bfd_malloc (o->rawsize);
-			      if (ocontents == NULL)
-				goto error_return;
-			      if (!bfd_get_section_contents (abfd, o, ocontents,
-							     (file_ptr) 0,
-							     o->rawsize))
+                  val = bfd_get_32 (abfd, ocontents + irelscan->r_offset);
+                  if (val != irelscan->r_addend) {
+			fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, irelscan->r_addend);
+                  }
+
+                  irelscan->r_addend -= calc_fixup (irelscan->r_addend, 0, sec);
+                  microblaze_bfd_write_imm_value_32 (abfd, ocontents + irelscan->r_offset,
+                                                     irelscan->r_addend);
+              }
+              if (ELF32_R_TYPE (irelscan->r_info) == (int) R_MICROBLAZE_32)
+                {
+	          isym = isymbuf + ELF32_R_SYM (irelscan->r_info);
+
+                  /* Look at the reloc only if the value has been resolved.  */
+                  if (isym->st_shndx == shndx
+                      && (ELF32_ST_TYPE (isym->st_info) == STT_SECTION))
+                    {
+                      if (ocontents == NULL)
+                        {
+                          if (elf_section_data (o)->this_hdr.contents != NULL)
+                            ocontents = elf_section_data (o)->this_hdr.contents;
+                          else
+                            {
+                              /* We always cache the section contents.
+                                 Perhaps, if info->keep_memory is FALSE, we
+                                 should free them, if we are permitted to.  */
+		              if (o->rawsize == 0)
+			        o->rawsize = o->size;
+                              ocontents = (bfd_byte *) bfd_malloc (o->rawsize);
+                              if (ocontents == NULL)
+                                goto error_return;
+                              if (!bfd_get_section_contents (abfd, o, ocontents,
+                                                             (file_ptr) 0,
+ 							      o->rawsize))
 				goto error_return;
 			      elf_section_data (o)->this_hdr.contents = ocontents;
 			    }
@@ -2068,7 +2137,7 @@ microblaze_elf_relax_section (bfd *abfd,
 			      elf_section_data (o)->this_hdr.contents = ocontents;
 			    }
 			}
-		      irelscan->r_addend -= calc_fixup (irel->r_addend
+		      irelscan->r_addend -= calc_fixup (irelscan->r_addend
 							+ isym->st_value,
 							0,
 							sec);
diff --git a/bfd/libbfd.h b/bfd/libbfd.h
index d5f42f22c08..d729dc48e7c 100644
--- a/bfd/libbfd.h
+++ b/bfd/libbfd.h
@@ -3010,6 +3010,7 @@ static const char *const bfd_reloc_code_real_names[] = { "@@uninitialized@@",
   "BFD_RELOC_MICROBLAZE_32_ROSDA",
   "BFD_RELOC_MICROBLAZE_32_RWSDA",
   "BFD_RELOC_MICROBLAZE_32_SYM_OP_SYM",
+  "BFD_RELOC_MICROBLAZE_32_NONE",
   "BFD_RELOC_MICROBLAZE_64_NONE",
   "BFD_RELOC_MICROBLAZE_64_GOTPC",
   "BFD_RELOC_MICROBLAZE_64_GOT",
diff --git a/bfd/reloc.c b/bfd/reloc.c
index 2ac883d0eac..3ea2afc0d4e 100644
--- a/bfd/reloc.c
+++ b/bfd/reloc.c
@@ -6694,6 +6694,12 @@ ENUM
 ENUMDOC
   This is a 32 bit reloc for the microblaze to handle
   expressions of the form "Symbol Op Symbol"
+ENUM
+  BFD_RELOC_MICROBLAZE_32_NONE
+ENUMDOC
+  This is a 32 bit reloc that stores the 32 bit pc relative
+  value in two words (with an imm instruction).  No relocation is
+  done here - only used for relaxing
 ENUM
   BFD_RELOC_MICROBLAZE_64_NONE
 ENUMDOC
diff --git a/binutils/readelf.c b/binutils/readelf.c
index c9b6210e229..57ac46e92b2 100644
--- a/binutils/readelf.c
+++ b/binutils/readelf.c
@@ -15279,6 +15279,10 @@ is_8bit_abs_reloc (Filedata * filedata, unsigned int reloc_type)
       return reloc_type == 54; /* R_RISCV_SET8.  */
     case EM_Z80:
       return reloc_type == 1;  /* R_Z80_8.  */
+    case EM_MICROBLAZE:
+      return reloc_type == 33 /* R_MICROBLAZE_32_NONE.  */
+             || reloc_type == 0 /* R_MICROBLAZE_NONE.  */
+             || reloc_type == 9; /* R_MICROBLAZE_64_NONE.  */
     default:
       return false;
     }
diff --git a/gas/config/tc-microblaze.c b/gas/config/tc-microblaze.c
index d900a9e1d05..98e91cda623 100644
--- a/gas/config/tc-microblaze.c
+++ b/gas/config/tc-microblaze.c
@@ -915,7 +915,7 @@ md_assemble (char * str)
   unsigned reg2;
   unsigned reg3;
   unsigned isize;
-  unsigned int immed = 0, temp;
+  unsigned int immed = 0, immed2 = 0, temp;
   expressionS exp;
   char name[20];
 
@@ -1176,6 +1176,77 @@ md_assemble (char * str)
       inst |= (reg2 << RA_LOW) & RA_MASK;
       inst |= (immed << IMM_LOW) & IMM5_MASK;
       break;
+      
+    case INST_TYPE_RD_R1_IMM5_IMM5:
+      if (strcmp (op_end, ""))
+        op_end = parse_reg (op_end + 1, &reg1);  /* Get rd.  */
+      else
+	{
+          as_fatal (_("Error in statement syntax"));
+          reg1 = 0;
+        }
+      if (strcmp (op_end, ""))
+        op_end = parse_reg (op_end + 1, &reg2);  /* Get r1.  */
+      else
+	{
+          as_fatal (_("Error in statement syntax"));
+          reg2 = 0;
+        }
+
+      /* Check for spl registers.  */
+      if (check_spl_reg (&reg1))
+        as_fatal (_("Cannot use special register with this instruction"));
+      if (check_spl_reg (&reg2))
+        as_fatal (_("Cannot use special register with this instruction"));
+ 
+      /* Width immediate value.  */
+      if (strcmp (op_end, ""))
+        op_end = parse_imm (op_end + 1, &exp, MIN_IMM_WIDTH, MAX_IMM_WIDTH);
+      else
+        as_fatal (_("Error in statement syntax"));
+      if (exp.X_op != O_constant)
+	{
+          as_warn (_("Symbol used as immediate width value for bit field instruction"));
+          immed = 1;
+        }
+      else
+        immed = exp.X_add_number;
+      if (opcode->instr == bsefi && immed > 31)
+        as_fatal (_("Width value must be less than 32"));
+
+      /* Shift immediate value.  */
+      if (strcmp (op_end, ""))
+        op_end = parse_imm (op_end + 1, &exp, MIN_IMM, MAX_IMM);
+      else
+        as_fatal (_("Error in statement syntax"));
+      if (exp.X_op != O_constant)
+	    {
+          as_warn (_("Symbol used as immediate shift value for bit field instruction"));
+          immed2 = 0;
+        }
+      else
+	    {
+          output = frag_more (isize);
+          immed2 = exp.X_add_number;
+	    }
+      if (immed2 != (immed2 % 32))
+	    {
+          as_warn (_("Shift value greater than 32. using <value %% 32>"));
+          immed2 = immed2 % 32;
+        }
+
+      /* Check combined value.  */
+      if (immed + immed2 > 32)
+        as_fatal (_("Width value + shift value must not be greater than 32"));
+
+      inst |= (reg1 << RD_LOW) & RD_MASK;
+      inst |= (reg2 << RA_LOW) & RA_MASK;
+      if (opcode->instr == bsefi)
+        inst |= (immed & IMM5_MASK) << IMM_WIDTH_LOW; /* bsefi */
+      else
+        inst |= ((immed + immed2 - 1) & IMM5_MASK) << IMM_WIDTH_LOW; /* bsifi */
+      inst |= (immed2 << IMM_LOW) & IMM5_MASK;
+      break;
 
     case INST_TYPE_R1_R2:
       if (strcmp (op_end, ""))
@@ -2209,9 +2280,12 @@ md_apply_fix (fixS *   fixP,
 	 moves code around due to relaxing.  */
       if (fixP->fx_r_type == BFD_RELOC_64_PCREL)
 	fixP->fx_r_type = BFD_RELOC_MICROBLAZE_64_NONE;
+      else if (fixP->fx_r_type == BFD_RELOC_32)
+	fixP->fx_r_type = BFD_RELOC_MICROBLAZE_32_NONE;	
       else
 	fixP->fx_r_type = BFD_RELOC_NONE;
       fixP->fx_addsy = section_symbol (absolute_section);
+      fixP->fx_done = 0;
     }
   return;
 }
@@ -2432,6 +2506,7 @@ tc_gen_reloc (asection * section ATTRIBUTE_UNUSED, fixS * fixp)
   switch (fixp->fx_r_type)
     {
     case BFD_RELOC_NONE:
+    case BFD_RELOC_MICROBLAZE_32_NONE:
     case BFD_RELOC_MICROBLAZE_64_NONE:
     case BFD_RELOC_32:
     case BFD_RELOC_MICROBLAZE_32_LO:
diff --git a/include/elf/microblaze.h b/include/elf/microblaze.h
index fecdd7e4831..164b36d0978 100644
--- a/include/elf/microblaze.h
+++ b/include/elf/microblaze.h
@@ -61,6 +61,7 @@ START_RELOC_NUMBERS (elf_microblaze_reloc_type)
   RELOC_NUMBER (R_MICROBLAZE_TEXTPCREL_64, 30)  /* PC-relative TEXT offset.  */
   RELOC_NUMBER (R_MICROBLAZE_TEXTREL_64, 31)    /* TEXT Entry offset 64-bit.  */
   RELOC_NUMBER (R_MICROBLAZE_TEXTREL_32_LO, 32) /* TEXT Entry offset 32-bit.  */
+  RELOC_NUMBER (R_MICROBLAZE_32_NONE, 33)
 END_RELOC_NUMBERS (R_MICROBLAZE_max)
 
 /* Global base address names.  */
diff --git a/opcodes/microblaze-dis.c b/opcodes/microblaze-dis.c
index 12981abfea1..e6419aab107 100644
--- a/opcodes/microblaze-dis.c
+++ b/opcodes/microblaze-dis.c
@@ -90,6 +90,18 @@ get_field_imm5_mbar (struct string_buf *buf, long instr)
   return p;
 }
 
+static char *
+get_field_imm5width (struct string_buf *buf, long instr)
+{
+  char *p = strbuf (buf);
+
+  if (instr & 0x00004000)
+    sprintf (p, "%d", (short)(((instr & IMM5_WIDTH_MASK) >> IMM_WIDTH_LOW))); /* bsefi */
+ else
+    sprintf (p, "%d", (short)(((instr & IMM5_WIDTH_MASK) >> IMM_WIDTH_LOW) - ((instr & IMM5_MASK) >> IMM_LOW) + 1)); /* bsifi */
+  return p;
+}
+
 static char *
 get_field_rfsl (struct string_buf *buf, long instr)
 {
@@ -427,6 +439,10 @@ print_insn_microblaze (bfd_vma memaddr, struct disassemble_info * info)
 	  /* For mbar 16 or sleep insn.  */
 	case INST_TYPE_NONE:
 	  break;
+        /* For bit field insns.  */
+	case INST_TYPE_RD_R1_IMM5_IMM5:
+	  print_func (stream, "\t%s, %s, %s, %s", get_field_rd (&buf, inst),get_field_r1(&buf, inst),get_field_imm5width (&buf, inst), get_field_imm5 (&buf, inst));
+	  break;	
 	  /* For tuqula instruction */
 	case INST_TYPE_RD:
 	  print_func (stream, "\t%s", get_field_rd (&buf, inst));
diff --git a/opcodes/microblaze-opc.h b/opcodes/microblaze-opc.h
index 7398e9e246a..63eb5644b0a 100644
--- a/opcodes/microblaze-opc.h
+++ b/opcodes/microblaze-opc.h
@@ -59,6 +59,9 @@
 /* For mbar.  */
 #define INST_TYPE_IMM5 20
 
+/* For bsefi and bsifi */
+#define INST_TYPE_RD_R1_IMM5_IMM5  21
+
 #define INST_TYPE_NONE 25
 
 
@@ -89,7 +92,9 @@
 #define OPCODE_MASK_H124  0xFFFF07FF /* High 16, and low 11 bits.  */
 #define OPCODE_MASK_H1234 0xFFFFFFFF /* All 32 bits.  */
 #define OPCODE_MASK_H3    0xFC000600 /* High 6 bits and bits 21, 22.  */
+#define OPCODE_MASK_H3B   0xFC00C600 /* High 6 bits and bits 16, 17, 21, 22.  */
 #define OPCODE_MASK_H32   0xFC00FC00 /* High 6 bits and bit 16-21.  */
+#define OPCODE_MASK_H32B  0xFC00C000 /* High 6 bits and bit 16, 17.  */
 #define OPCODE_MASK_H34B  0xFC0000FF /* High 6 bits and low 8 bits.  */
 #define OPCODE_MASK_H35B  0xFC0004FF /* High 6 bits and low 9 bits.  */
 #define OPCODE_MASK_H34C  0xFC0007E0 /* High 6 bits and bits 21-26.  */
@@ -102,7 +107,7 @@
 #define DELAY_SLOT 1
 #define NO_DELAY_SLOT 0
 
-#define MAX_OPCODES 300
+#define MAX_OPCODES 301
 
 const struct op_code_struct
 {
@@ -159,6 +164,8 @@ const struct op_code_struct
   {"bslli", INST_TYPE_RD_R1_IMM5, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x64000400, OPCODE_MASK_H3, bslli, barrel_shift_inst },
   {"bsrai", INST_TYPE_RD_R1_IMM5, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x64000200, OPCODE_MASK_H3, bsrai, barrel_shift_inst },
   {"bsrli", INST_TYPE_RD_R1_IMM5, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x64000000, OPCODE_MASK_H3, bsrli, barrel_shift_inst },
+  {"bsefi", INST_TYPE_RD_R1_IMM5_IMM5, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x64004000, OPCODE_MASK_H32B, bsefi, barrel_shift_inst },
+  {"bsifi", INST_TYPE_RD_R1_IMM5_IMM5, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x64008000, OPCODE_MASK_H32B, bsifi, barrel_shift_inst },
   {"or",    INST_TYPE_RD_R1_R2, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x80000000, OPCODE_MASK_H4, microblaze_or, logical_inst },
   {"and",   INST_TYPE_RD_R1_R2, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x84000000, OPCODE_MASK_H4, microblaze_and, logical_inst },
   {"xor",   INST_TYPE_RD_R1_R2, INST_NO_OFFSET, NO_DELAY_SLOT, IMMVAL_MASK_NON_SPECIAL, 0x88000000, OPCODE_MASK_H4, microblaze_xor, logical_inst },
@@ -438,5 +445,8 @@ char pvr_register_prefix[] = "rpvr";
 #define MIN_IMM5  ((int) 0x00000000)
 #define MAX_IMM5  ((int) 0x0000001f)
 
+#define MIN_IMM_WIDTH  ((int) 0x00000001)
+#define MAX_IMM_WIDTH  ((int) 0x00000020)
+
 #endif /* MICROBLAZE_OPC */
 
diff --git a/opcodes/microblaze-opcm.h b/opcodes/microblaze-opcm.h
index c91b002d951..3c4f8948c76 100644
--- a/opcodes/microblaze-opcm.h
+++ b/opcodes/microblaze-opcm.h
@@ -29,7 +29,7 @@ enum microblaze_instr
   addi, rsubi, addic, rsubic, addik, rsubik, addikc, rsubikc, mul,
   mulh, mulhu, mulhsu, swapb, swaph,
   idiv, idivu, bsll, bsra, bsrl, get, put, nget, nput, cget, cput,
-  ncget, ncput, muli, bslli, bsrai, bsrli, mului,
+  ncget, ncput, muli, bslli, bsrai, bsrli, bsefi, bsifi, mului,
   /* 'or/and/xor' are C++ keywords.  */
   microblaze_or, microblaze_and, microblaze_xor,
   andn, pcmpbf, pcmpbc, pcmpeq, pcmpne, sra, src, srl, sext8, sext16,
@@ -130,6 +130,7 @@ enum microblaze_instr_type
 #define RB_LOW  11 /* Low bit for RB.  */
 #define IMM_LOW  0 /* Low bit for immediate.  */
 #define IMM_MBAR 21 /* low bit for mbar instruction.  */
+#define IMM_WIDTH_LOW 6 /* Low bit for immediate width */
 
 #define RD_MASK 0x03E00000
 #define RA_MASK 0x001F0000
@@ -142,6 +143,9 @@ enum microblaze_instr_type
 /* Imm mask for mbar.  */
 #define IMM5_MBAR_MASK 0x03E00000
 
+/* Imm mask for extract/insert width. */
+#define IMM5_WIDTH_MASK 0x000007C0
+
 /* FSL imm mask for get, put instructions.  */
 #define  RFSL_MASK 0x000000F
 
-- 
2.25.1

Re: [PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions

[Michael Eager]

On 10/5/23 05:51, Neal Frager via Binutils wrote:
> This patches adds new bsefi and bsifi instructions.
> BSEFI- The instruction shall extract a bit field from a
> register and place it right-adjusted in the destination register.
> The other bits in the destination register shall be set to zero.
> BSIFI- The instruction shall insert a right-adjusted bit field
> from a register at another position in the destination register.
> The rest of the bits in the destination register shall be unchanged.
> 
> Further documentation of these instructions can be found here:
> https://docs.xilinx.com/v/u/en-US/ug984-vivado-microblaze-ref
> 
> This patch has been tested for years of AMD Xilinx Yocto
> releases as part of the following patch set:
> 
> https://github.com/Xilinx/meta-xilinx/tree/master/meta-microblaze/recipes-devtools/binutils/binutils
> 
> Signed-off-by: nagaraju <nagaraju.mekala@amd.com>
> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@amd.com>
> Signed-off-by: Neal Frager <neal.frager@amd.com>

Committed.

Please add test cases for these instructions.

-- 
Michael Eager

Re: [PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions

[Maciej W. Rozycki]

On Fri, 6 Oct 2023, Michael Eager wrote:

> > This patch has been tested for years of AMD Xilinx Yocto
> > releases as part of the following patch set:
> > 
> > https://github.com/Xilinx/meta-xilinx/tree/master/meta-microblaze/recipes-devtools/binutils/binutils
> > 
> > Signed-off-by: nagaraju <nagaraju.mekala@amd.com>
> > Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@amd.com>
> > Signed-off-by: Neal Frager <neal.frager@amd.com>
> 
> Committed.

 Yet it has caused numerous regressions:

microblaze-elf  +FAIL: unordered .debug_info references to .debug_ranges
microblaze-elf  +FAIL: binutils-all/pr26548
microblaze-elf  +FAIL: readelf -Wwi pr26548e (reason: unexpected output)
microblaze-elf  +FAIL: readelf --debug-dump=loc locview-1 (reason: unexpected output)
microblaze-elf  +FAIL: readelf --debug-dump=loc locview-2 (reason: unexpected output)
microblaze-elf  +FAIL: readelf -wiaoRlL dw5
microblaze-elf  +FAIL: readelf -wi dwarf-attributes (reason: unexpected output)
microblaze-elf  +FAIL: readelf -wKis -P debuglink (reason: unexpected output)
microblaze-elf  +FAIL: readelf --debug-dump=links --debug-dump=no-follow-links dwo
microblaze-elf  +FAIL: DWARF2 1
microblaze-elf  +FAIL: DWARF2 2
microblaze-elf  +FAIL: DWARF2 3
microblaze-elf  +FAIL: DWARF2 5
microblaze-elf  +FAIL: DWARF2 6
microblaze-elf  +FAIL: DWARF2 7
microblaze-elf  +FAIL: DWARF2 11
microblaze-elf  +FAIL: DWARF2 12
microblaze-elf  +FAIL: DWARF2 13
microblaze-elf  +FAIL: DWARF2 14
microblaze-elf  +FAIL: DWARF2 15
microblaze-elf  +FAIL: DWARF2 16
microblaze-elf  +FAIL: DWARF2 17
microblaze-elf  +FAIL: DWARF2 18
microblaze-elf  +FAIL: DWARF2 19
microblaze-elf  +FAIL: DWARF2_20: debug ranges ignore non-code sections
microblaze-elf  +FAIL: DWARF2 21
microblaze-elf  +FAIL: DWARF5 .loc 0
microblaze-elf  +FAIL: DWARF4 CU
microblaze-elf  +FAIL: DWARF5 CU
microblaze-elf  +FAIL: Check line table is produced with .nops
microblaze-elf  +FAIL: line number entries for section changes inside .irp
microblaze-elf  +FAIL: line number entries for .macro expansions
microblaze-elf  +FAIL: line number entries for expansions of .macro coming from .include
microblaze-elf  +FAIL: lns-duplicate
microblaze-elf  +FAIL: lns-common-1
microblaze-elf  +FAIL: ld-elf/pr22450

They all seem to follow a similar pattern, e.g:

exited abnormally with 1, output:readelf: Warning: Corrupt unit length (got 0x4e00004e expected at most 0x4e) in section .debug_info

FAIL: DWARF2 1

or:

exited abnormally with 0, output:readelf: Warning: Corrupt unit length (got 0x20000020 expected at most 0x20) in section .debug_info

FAIL: Check line table is produced with .nops

Configured with:

$ /path/to/configure --disable-nls --disable-gdb --disable-gdbserver --disable-gprofng --disable-libbacktrace --disable-libdecnumber --disable-readline --disable-sim --enable-obsolete --enable-plugins --build=powerpc64le-linux --target=microblaze-elf

 Just reporting in case it's useful as it has popped up in unrelated 
verification; I won't do anything else here.

 NB I've skimmed over the change and noticed it does not follow the GNU 
Coding Standards in many places, most prominently in `get_field_imm5width' 
and `get_field_rfsl', but also elsewhere (which is easy to spot looking 
through the diff).

  Maciej

[PATCH] microblaze: fix build error on 32-bit hosts

[Mark Wielaard]

commit 6bbf24955 opcodes: microblaze: Add new bit-field instructions
introduced a build error on 32-bit hosts:

elf32-microblaze.c: In function ‘microblaze_elf_relax_section’:
elf32-microblaze.c:1989:53: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka ‘unsigned int’} [-Werror=format=]
 1989 |       fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, irel->r_addend);
      |                                                   ~~^                    ~~~~~~~~~~~~~~
      |                                                     |                        |
      |                                                     long unsigned int        bfd_vma {aka unsigned int}
      |                                                   %x
elf32-microblaze.c:2074:51: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka ‘unsigned int’} [-Werror=format=]
 2074 |    fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, irelscan->r_addend);
      |                                                 ~~^                    ~~~~~~~~~~~~~~~~~~
      |                                                   |                            |
      |                                                   long unsigned int            bfd_vma {aka unsigned int}
      |                                                 %x

Fix by explicitly casting the r_addend to long.
---
 bfd/elf32-microblaze.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bfd/elf32-microblaze.c b/bfd/elf32-microblaze.c
index a8ced43c08a..2c584f91a4e 100644
--- a/bfd/elf32-microblaze.c
+++ b/bfd/elf32-microblaze.c
@@ -1986,7 +1986,7 @@ microblaze_elf_relax_section (bfd *abfd,
 		/* Validate the in-band val.  */
 		val = bfd_get_32 (abfd, contents + irel->r_offset);
 		if (val != irel->r_addend && ELF32_R_TYPE (irel->r_info) == R_MICROBLAZE_32_NONE) {
-		    fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, irel->r_addend);
+		    fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, (long) irel->r_addend);
 		}
 		irel->r_addend -= (efix - sfix);
 		/* Should use HOWTO.  */
@@ -2071,7 +2071,7 @@ microblaze_elf_relax_section (bfd *abfd,
 
 		  val = bfd_get_32 (abfd, ocontents + irelscan->r_offset);
 		  if (val != irelscan->r_addend) {
-			fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, irelscan->r_addend);
+			fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, (long) irelscan->r_addend);
 		  }
 
 		  irelscan->r_addend -= calc_fixup (irelscan->r_addend, 0, sec);
-- 
2.39.3

Re: [PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions

[Michael Eager]

Reverted patch.

On 10/7/23 12:23, Maciej W. Rozycki wrote:
> On Fri, 6 Oct 2023, Michael Eager wrote:
> 
>>> This patch has been tested for years of AMD Xilinx Yocto
>>> releases as part of the following patch set:
>>>
>>> https://github.com/Xilinx/meta-xilinx/tree/master/meta-microblaze/recipes-devtools/binutils/binutils
>>>
>>> Signed-off-by: nagaraju <nagaraju.mekala@amd.com>
>>> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@amd.com>
>>> Signed-off-by: Neal Frager <neal.frager@amd.com>
>>
>> Committed.
> 
>   Yet it has caused numerous regressions:
> 
> microblaze-elf  +FAIL: unordered .debug_info references to .debug_ranges
> microblaze-elf  +FAIL: binutils-all/pr26548
> microblaze-elf  +FAIL: readelf -Wwi pr26548e (reason: unexpected output)
> microblaze-elf  +FAIL: readelf --debug-dump=loc locview-1 (reason: unexpected output)
> microblaze-elf  +FAIL: readelf --debug-dump=loc locview-2 (reason: unexpected output)
> microblaze-elf  +FAIL: readelf -wiaoRlL dw5
> microblaze-elf  +FAIL: readelf -wi dwarf-attributes (reason: unexpected output)
> microblaze-elf  +FAIL: readelf -wKis -P debuglink (reason: unexpected output)
> microblaze-elf  +FAIL: readelf --debug-dump=links --debug-dump=no-follow-links dwo
> microblaze-elf  +FAIL: DWARF2 1
> microblaze-elf  +FAIL: DWARF2 2
> microblaze-elf  +FAIL: DWARF2 3
> microblaze-elf  +FAIL: DWARF2 5
> microblaze-elf  +FAIL: DWARF2 6
> microblaze-elf  +FAIL: DWARF2 7
> microblaze-elf  +FAIL: DWARF2 11
> microblaze-elf  +FAIL: DWARF2 12
> microblaze-elf  +FAIL: DWARF2 13
> microblaze-elf  +FAIL: DWARF2 14
> microblaze-elf  +FAIL: DWARF2 15
> microblaze-elf  +FAIL: DWARF2 16
> microblaze-elf  +FAIL: DWARF2 17
> microblaze-elf  +FAIL: DWARF2 18
> microblaze-elf  +FAIL: DWARF2 19
> microblaze-elf  +FAIL: DWARF2_20: debug ranges ignore non-code sections
> microblaze-elf  +FAIL: DWARF2 21
> microblaze-elf  +FAIL: DWARF5 .loc 0
> microblaze-elf  +FAIL: DWARF4 CU
> microblaze-elf  +FAIL: DWARF5 CU
> microblaze-elf  +FAIL: Check line table is produced with .nops
> microblaze-elf  +FAIL: line number entries for section changes inside .irp
> microblaze-elf  +FAIL: line number entries for .macro expansions
> microblaze-elf  +FAIL: line number entries for expansions of .macro coming from .include
> microblaze-elf  +FAIL: lns-duplicate
> microblaze-elf  +FAIL: lns-common-1
> microblaze-elf  +FAIL: ld-elf/pr22450
> 
> They all seem to follow a similar pattern, e.g:
> 
> exited abnormally with 1, output:readelf: Warning: Corrupt unit length (got 0x4e00004e expected at most 0x4e) in section .debug_info
> 
> FAIL: DWARF2 1
> 
> or:
> 
> exited abnormally with 0, output:readelf: Warning: Corrupt unit length (got 0x20000020 expected at most 0x20) in section .debug_info
> 
> FAIL: Check line table is produced with .nops
> 
> Configured with:
> 
> $ /path/to/configure --disable-nls --disable-gdb --disable-gdbserver --disable-gprofng --disable-libbacktrace --disable-libdecnumber --disable-readline --disable-sim --enable-obsolete --enable-plugins --build=powerpc64le-linux --target=microblaze-elf
> 
>   Just reporting in case it's useful as it has popped up in unrelated
> verification; I won't do anything else here.
> 
>   NB I've skimmed over the change and noticed it does not follow the GNU
> Coding Standards in many places, most prominently in `get_field_imm5width'
> and `get_field_rfsl', but also elsewhere (which is easy to spot looking
> through the diff).
> 
>    Maciej
> 

-- 
Michael Eager

Re: [PATCH] microblaze: fix build error on 32-bit hosts

[Michael Eager]

On 10/7/23 15:01, Mark Wielaard wrote:
> commit 6bbf24955 opcodes: microblaze: Add new bit-field instructions
> introduced a build error on 32-bit hosts:
> 
> elf32-microblaze.c: In function ‘microblaze_elf_relax_section’:
> elf32-microblaze.c:1989:53: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka ‘unsigned int’} [-Werror=format=]
>   1989 |       fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, irel->r_addend);
>        |                                                   ~~^                    ~~~~~~~~~~~~~~
>        |                                                     |                        |
>        |                                                     long unsigned int        bfd_vma {aka unsigned int}
>        |                                                   %x
> elf32-microblaze.c:2074:51: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka ‘unsigned int’} [-Werror=format=]
>   2074 |    fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, irelscan->r_addend);
>        |                                                 ~~^                    ~~~~~~~~~~~~~~~~~~
>        |                                                   |                            |
>        |                                                   long unsigned int            bfd_vma {aka unsigned int}
>        |                                                 %x
> 
> Fix by explicitly casting the r_addend to long.
> ---
>   bfd/elf32-microblaze.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/bfd/elf32-microblaze.c b/bfd/elf32-microblaze.c
> index a8ced43c08a..2c584f91a4e 100644
> --- a/bfd/elf32-microblaze.c
> +++ b/bfd/elf32-microblaze.c
> @@ -1986,7 +1986,7 @@ microblaze_elf_relax_section (bfd *abfd,
>   		/* Validate the in-band val.  */
>   		val = bfd_get_32 (abfd, contents + irel->r_offset);
>   		if (val != irel->r_addend && ELF32_R_TYPE (irel->r_info) == R_MICROBLAZE_32_NONE) {
> -		    fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, irel->r_addend);
> +		    fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, (long) irel->r_addend);
>   		}
>   		irel->r_addend -= (efix - sfix);
>   		/* Should use HOWTO.  */
> @@ -2071,7 +2071,7 @@ microblaze_elf_relax_section (bfd *abfd,
>   
>   		  val = bfd_get_32 (abfd, ocontents + irelscan->r_offset);
>   		  if (val != irelscan->r_addend) {
> -			fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, irelscan->r_addend);
> +			fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, (long) irelscan->r_addend);
>   		  }
>   
>   		  irelscan->r_addend -= calc_fixup (irelscan->r_addend, 0, sec);

I didn't see any build errors, building with GCC-12.3.1.

Which version of GCC are you using?

(Patch was reverted.)

-- 
Michael Eager

Re: [PATCH] microblaze: fix build error on 32-bit hosts

[Michael Eager]

On 10/7/23 15:53, Michael Eager wrote:
> On 10/7/23 15:01, Mark Wielaard wrote:
>> commit 6bbf24955 opcodes: microblaze: Add new bit-field instructions
>> introduced a build error on 32-bit hosts:
>>
>> elf32-microblaze.c: In function ‘microblaze_elf_relax_section’:
>> elf32-microblaze.c:1989:53: error: format ‘%lx’ expects argument of 
>> type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka 
>> ‘unsigned int’} [-Werror=format=]
>>   1989 |       fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", 
>> __LINE__, val, irel->r_addend);
>>        |                                                   
>> ~~^                    ~~~~~~~~~~~~~~
>>        |                                                     
>> |                        |
>>        |                                                     long 
>> unsigned int        bfd_vma {aka unsigned int}
>>        |                                                   %x
>> elf32-microblaze.c:2074:51: error: format ‘%lx’ expects argument of 
>> type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka 
>> ‘unsigned int’} [-Werror=format=]
>>   2074 |    fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", 
>> __LINE__, val, irelscan->r_addend);
>>        |                                                 
>> ~~^                    ~~~~~~~~~~~~~~~~~~
>>        |                                                   
>> |                            |
>>        |                                                   long 
>> unsigned int            bfd_vma {aka unsigned int}
>>        |                                                 %x
>>
>> Fix by explicitly casting the r_addend to long.
>> ---
>>   bfd/elf32-microblaze.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/bfd/elf32-microblaze.c b/bfd/elf32-microblaze.c
>> index a8ced43c08a..2c584f91a4e 100644
>> --- a/bfd/elf32-microblaze.c
>> +++ b/bfd/elf32-microblaze.c
>> @@ -1986,7 +1986,7 @@ microblaze_elf_relax_section (bfd *abfd,
>>           /* Validate the in-band val.  */
>>           val = bfd_get_32 (abfd, contents + irel->r_offset);
>>           if (val != irel->r_addend && ELF32_R_TYPE (irel->r_info) == 
>> R_MICROBLAZE_32_NONE) {
>> -            fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", 
>> __LINE__, val, irel->r_addend);
>> +            fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", 
>> __LINE__, val, (long) irel->r_addend);
>>           }
>>           irel->r_addend -= (efix - sfix);
>>           /* Should use HOWTO.  */
>> @@ -2071,7 +2071,7 @@ microblaze_elf_relax_section (bfd *abfd,
>>             val = bfd_get_32 (abfd, ocontents + irelscan->r_offset);
>>             if (val != irelscan->r_addend) {
>> -            fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", 
>> __LINE__, val, irelscan->r_addend);
>> +            fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", 
>> __LINE__, val, (long) irelscan->r_addend);
>>             }
>>             irelscan->r_addend -= calc_fixup (irelscan->r_addend, 0, 
>> sec);
> 
> I didn't see any build errors, building with GCC-12.3.1.

Building with GCC-13.2.1 on Fedora 38 -- no error messages.

> Which version of GCC are you using?
> 
> (Patch was reverted.)
> 

-- 
Michael Eager

Re: [PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions

[Michael Eager]

Neal --

Please resubmit this patch.

Add test cases for new instructions.
Fix GNU Coding Standard issues.
Run binutils test suite.

Thanks.

On 10/7/23 12:23, Maciej W. Rozycki wrote:
> On Fri, 6 Oct 2023, Michael Eager wrote:
> 
>>> This patch has been tested for years of AMD Xilinx Yocto
>>> releases as part of the following patch set:
>>>
>>> https://github.com/Xilinx/meta-xilinx/tree/master/meta-microblaze/recipes-devtools/binutils/binutils
>>>
>>> Signed-off-by: nagaraju <nagaraju.mekala@amd.com>
>>> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@amd.com>
>>> Signed-off-by: Neal Frager <neal.frager@amd.com>
>>
>> Committed.
> 
>   Yet it has caused numerous regressions:
> 
> microblaze-elf  +FAIL: unordered .debug_info references to .debug_ranges
> microblaze-elf  +FAIL: binutils-all/pr26548
> microblaze-elf  +FAIL: readelf -Wwi pr26548e (reason: unexpected output)
> microblaze-elf  +FAIL: readelf --debug-dump=loc locview-1 (reason: unexpected output)
> microblaze-elf  +FAIL: readelf --debug-dump=loc locview-2 (reason: unexpected output)
> microblaze-elf  +FAIL: readelf -wiaoRlL dw5
> microblaze-elf  +FAIL: readelf -wi dwarf-attributes (reason: unexpected output)
> microblaze-elf  +FAIL: readelf -wKis -P debuglink (reason: unexpected output)
> microblaze-elf  +FAIL: readelf --debug-dump=links --debug-dump=no-follow-links dwo
> microblaze-elf  +FAIL: DWARF2 1
> microblaze-elf  +FAIL: DWARF2 2
> microblaze-elf  +FAIL: DWARF2 3
> microblaze-elf  +FAIL: DWARF2 5
> microblaze-elf  +FAIL: DWARF2 6
> microblaze-elf  +FAIL: DWARF2 7
> microblaze-elf  +FAIL: DWARF2 11
> microblaze-elf  +FAIL: DWARF2 12
> microblaze-elf  +FAIL: DWARF2 13
> microblaze-elf  +FAIL: DWARF2 14
> microblaze-elf  +FAIL: DWARF2 15
> microblaze-elf  +FAIL: DWARF2 16
> microblaze-elf  +FAIL: DWARF2 17
> microblaze-elf  +FAIL: DWARF2 18
> microblaze-elf  +FAIL: DWARF2 19
> microblaze-elf  +FAIL: DWARF2_20: debug ranges ignore non-code sections
> microblaze-elf  +FAIL: DWARF2 21
> microblaze-elf  +FAIL: DWARF5 .loc 0
> microblaze-elf  +FAIL: DWARF4 CU
> microblaze-elf  +FAIL: DWARF5 CU
> microblaze-elf  +FAIL: Check line table is produced with .nops
> microblaze-elf  +FAIL: line number entries for section changes inside .irp
> microblaze-elf  +FAIL: line number entries for .macro expansions
> microblaze-elf  +FAIL: line number entries for expansions of .macro coming from .include
> microblaze-elf  +FAIL: lns-duplicate
> microblaze-elf  +FAIL: lns-common-1
> microblaze-elf  +FAIL: ld-elf/pr22450
> 
> They all seem to follow a similar pattern, e.g:
> 
> exited abnormally with 1, output:readelf: Warning: Corrupt unit length (got 0x4e00004e expected at most 0x4e) in section .debug_info
> 
> FAIL: DWARF2 1
> 
> or:
> 
> exited abnormally with 0, output:readelf: Warning: Corrupt unit length (got 0x20000020 expected at most 0x20) in section .debug_info
> 
> FAIL: Check line table is produced with .nops
> 
> Configured with:
> 
> $ /path/to/configure --disable-nls --disable-gdb --disable-gdbserver --disable-gprofng --disable-libbacktrace --disable-libdecnumber --disable-readline --disable-sim --enable-obsolete --enable-plugins --build=powerpc64le-linux --target=microblaze-elf
> 
>   Just reporting in case it's useful as it has popped up in unrelated
> verification; I won't do anything else here.
> 
>   NB I've skimmed over the change and noticed it does not follow the GNU
> Coding Standards in many places, most prominently in `get_field_imm5width'
> and `get_field_rfsl', but also elsewhere (which is easy to spot looking
> through the diff).
> 
>    Maciej
> 

-- 
Michael Eager

Re: [PATCH] microblaze: fix build error on 32-bit hosts

[Mark Wielaard]

Hi,

On Sat, Oct 07, 2023 at 04:07:30PM -0700, Michael Eager wrote:
> 
> Building with GCC-13.2.1 on Fedora 38 -- no error messages.
> 
> >Which version of GCC are you using?

gcc (Debian 10.2.1-6) 10.2.1 20210110 on debian-i386.

But I don't think it depends on the gcc version.  It depends on having
an 32bit host that is configured with --enable-targets=all.

Cheers,

Mark

RE: [PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions

[Frager, Neal]

Hi Michael, Maciej,

> Neal --

> Please resubmit this patch.

> Add test cases for new instructions.
> Fix GNU Coding Standard issues.
> Run binutils test suite.

Thank you for feedback.  Could you possibly help me with resolving the GNU coding standard issues and the build issue Maciej is reporting?

Currently, the problem we are facing is that the upstream binutils is unable to build the zynqmp pmufw application.  I am trying to resolve this by upstreaming the toolchain patches AMD is currently using in its microblaze toolchain release.

This is the only patch remaining that is preventing the zynqmp pmufw application from building, as the application needs the microblaze bsefi and bsifi instructions.

My patch for adding these instructions is coming from the following two patches which AMD currently applies on its own microblaze toolchain that comes with our yocto releases:
https://github.com/Xilinx/meta-xilinx/blob/master/meta-microblaze/recipes-devtools/binutils/binutils/0004-LOCAL-Fix-relaxation-of-assembler-resolved-reference.patch
https://github.com/Xilinx/meta-xilinx/blob/master/meta-microblaze/recipes-devtools/binutils/binutils/0008-Add-new-bit-field-instructions.patch

As I am far away from being a toolchain or GNU binutils expert, could you please help me in the simplest of terms understand what is not following the GNU coding standard in these patches, so that I can fix it?

My current way of testing is verifying that the master branch build of GNU binutils is able to build the zynqmp pmufw application and that it executes correctly on zynqmp hardware.  So I have verified on hardware that the v2 version of the patch I submitted is indeed working.  If you could help me with resolving these regressions, so that this patch can solve the problem I am trying to solve without causing another problem, I would appreciate any support you can provide.

Also, are there any instructions for coding standard for generating test cases for the bsefi and bsifi instructions?  As I have never written GNU binutils test cases before, perhaps you can point me to something that will help me to save time with this as well?

Thank you for your support!

Best regards,
Neal Frager
AMD

> On Fri, 6 Oct 2023, Michael Eager wrote:
> 
>>> This patch has been tested for years of AMD Xilinx Yocto releases as 
>>> part of the following patch set:
>>>
>>> https://github.com/Xilinx/meta-xilinx/tree/master/meta-microblaze/re
>>> cipes-devtools/binutils/binutils
>>>
>>> Signed-off-by: nagaraju <nagaraju.mekala@amd.com>
>>> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@amd.com>
>>> Signed-off-by: Neal Frager <neal.frager@amd.com>
>>
>> Committed.
> 
>   Yet it has caused numerous regressions:
> 
> microblaze-elf  +FAIL: unordered .debug_info references to 
> .debug_ranges microblaze-elf  +FAIL: binutils-all/pr26548 
> microblaze-elf  +FAIL: readelf -Wwi pr26548e (reason: unexpected 
> output) microblaze-elf  +FAIL: readelf --debug-dump=loc locview-1 
> (reason: unexpected output) microblaze-elf  +FAIL: readelf 
> --debug-dump=loc locview-2 (reason: unexpected output) microblaze-elf  
> +FAIL: readelf -wiaoRlL dw5 microblaze-elf  +FAIL: readelf -wi 
> dwarf-attributes (reason: unexpected output) microblaze-elf  +FAIL: 
> readelf -wKis -P debuglink (reason: unexpected output) microblaze-elf  
> +FAIL: readelf --debug-dump=links --debug-dump=no-follow-links dwo 
> microblaze-elf  +FAIL: DWARF2 1 microblaze-elf  +FAIL: DWARF2 2 
> microblaze-elf  +FAIL: DWARF2 3 microblaze-elf  +FAIL: DWARF2 5 
> microblaze-elf  +FAIL: DWARF2 6 microblaze-elf  +FAIL: DWARF2 7 
> microblaze-elf  +FAIL: DWARF2 11 microblaze-elf  +FAIL: DWARF2 12 
> microblaze-elf  +FAIL: DWARF2 13 microblaze-elf  +FAIL: DWARF2 14 
> microblaze-elf  +FAIL: DWARF2 15 microblaze-elf  +FAIL: DWARF2 16 
> microblaze-elf  +FAIL: DWARF2 17 microblaze-elf  +FAIL: DWARF2 18 
> microblaze-elf  +FAIL: DWARF2 19 microblaze-elf  +FAIL: DWARF2_20: 
> debug ranges ignore non-code sections microblaze-elf  +FAIL: DWARF2 21 
> microblaze-elf  +FAIL: DWARF5 .loc 0 microblaze-elf  +FAIL: DWARF4 CU 
> microblaze-elf  +FAIL: DWARF5 CU microblaze-elf  +FAIL: Check line 
> table is produced with .nops microblaze-elf  +FAIL: line number 
> entries for section changes inside .irp microblaze-elf  +FAIL: line 
> number entries for .macro expansions microblaze-elf  +FAIL: line 
> number entries for expansions of .macro coming from .include 
> microblaze-elf  +FAIL: lns-duplicate microblaze-elf  +FAIL: 
> lns-common-1 microblaze-elf  +FAIL: ld-elf/pr22450
> 
> They all seem to follow a similar pattern, e.g:
> 
> exited abnormally with 1, output:readelf: Warning: Corrupt unit length 
> (got 0x4e00004e expected at most 0x4e) in section .debug_info
> 
> FAIL: DWARF2 1
> 
> or:
> 
> exited abnormally with 0, output:readelf: Warning: Corrupt unit length 
> (got 0x20000020 expected at most 0x20) in section .debug_info
> 
> FAIL: Check line table is produced with .nops
> 
> Configured with:
> 
> $ /path/to/configure --disable-nls --disable-gdb --disable-gdbserver 
> --disable-gprofng --disable-libbacktrace --disable-libdecnumber 
> --disable-readline --disable-sim --enable-obsolete --enable-plugins 
> --build=powerpc64le-linux --target=microblaze-elf
> 
>   Just reporting in case it's useful as it has popped up in unrelated 
> verification; I won't do anything else here.
> 
>   NB I've skimmed over the change and noticed it does not follow the 
> GNU Coding Standards in many places, most prominently in `get_field_imm5width'
> and `get_field_rfsl', but also elsewhere (which is easy to spot 
> looking through the diff).
> 
>    Maciej
> 

> --
> Michael Eager

RE: [PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions

[Frager, Neal]

> > This patch has been tested for years of AMD Xilinx Yocto releases as 
> > part of the following patch set:
> > 
> > https://github.com/Xilinx/meta-xilinx/tree/master/meta-microblaze/re
> > cipes-devtools/binutils/binutils
> > 
> > Signed-off-by: nagaraju <nagaraju.mekala@amd.com>
> > Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@amd.com>
> > Signed-off-by: Neal Frager <neal.frager@amd.com>
> 
> Committed.

> Yet it has caused numerous regressions:

> microblaze-elf  +FAIL: unordered .debug_info references to .debug_ranges microblaze-elf  +FAIL: binutils-all/pr26548 microblaze-elf  +FAIL: readelf -Wwi pr26548e (reason: unexpected output) microblaze-elf  +FAIL: readelf --debug-dump=loc locview-1 (reason: unexpected output) microblaze-elf  +FAIL: readelf --debug-dump=loc locview-2 (reason: unexpected output) microblaze-elf  +FAIL: readelf -wiaoRlL dw5 microblaze-elf  +FAIL: readelf -wi dwarf-attributes (reason: unexpected output) microblaze-elf  +FAIL: readelf -wKis -P debuglink (reason: unexpected output) microblaze-elf  +FAIL: readelf --debug-dump=links --debug-dump=no-follow-links dwo microblaze-elf  +FAIL: DWARF2 1 microblaze-elf  +FAIL: DWARF2 2 microblaze-elf  +FAIL: DWARF2 3 microblaze-elf  +FAIL: DWARF2 5 microblaze-elf  +FAIL: DWARF2 6 microblaze-elf  +FAIL: DWARF2 7 microblaze-elf  +FAIL: DWARF2 11 microblaze-elf  +FAIL: DWARF2 12 microblaze-elf  +FAIL: DWARF2 13 microblaze-elf  +FAIL: DWARF2 14 microblaze-elf  +FAIL: DWARF2 15 microblaze-elf  +FAIL: DWARF2 16 microblaze-elf  +FAIL: DWARF2 17 microblaze-elf  +FAIL: DWARF2 18 microblaze-elf  +FAIL: DWARF2 19 microblaze-elf  +FAIL: DWARF2_20: debug ranges ignore non-code sections microblaze-elf  +FAIL: DWARF2 21 microblaze-elf  +FAIL: DWARF5 .loc 0 microblaze-elf  +FAIL: DWARF4 CU microblaze-elf  +FAIL: DWARF5 CU microblaze-elf  +FAIL: Check line table is produced with .nops microblaze-elf  +FAIL: line number entries for section changes inside .irp microblaze-elf  +FAIL: line number entries for .macro expansions microblaze-elf  +FAIL: line number entries for expansions of .macro coming from .include microblaze-elf  +FAIL: lns-duplicate microblaze-elf  +FAIL: lns-common-1 microblaze-elf  +FAIL: ld-elf/pr22450

> They all seem to follow a similar pattern, e.g:

> exited abnormally with 1, output:readelf: Warning: Corrupt unit length (got 0x4e00004e expected at most 0x4e) in section .debug_info

> FAIL: DWARF2 1

> or:

> exited abnormally with 0, output:readelf: Warning: Corrupt unit length (got 0x20000020 expected at most 0x20) in section .debug_info

> FAIL: Check line table is produced with .nops

> Configured with:

> $ /path/to/configure --disable-nls --disable-gdb --disable-gdbserver --disable-gprofng --disable-libbacktrace --disable-libdecnumber --disable-readline --disable-sim --enable-obsolete --enable-plugins --build=powerpc64le-linux --target=microblaze-elf

> Just reporting in case it's useful as it has popped up in unrelated verification; I won't do anything else here.

> NB I've skimmed over the change and noticed it does not follow the GNU Coding Standards in many places, most prominently in `get_field_imm5width' 
and `get_field_rfsl', but also elsewhere (which is easy to spot looking through the diff).

Hi Maciej,

The patch I submitted does not modify the 'get_field_rfsl' function, but perhaps you could review
the 'get_field_imm5width' function?  Could you explain what is not following the GNU Coding
Standards, so that I can fix it?

static char *
get_field_imm5width (struct string_buf *buf, long instr)
{
  char *p = strbuf (buf);

  if (instr & 0x00004000)
    sprintf (p, "%d", (short)(((instr & IMM5_WIDTH_MASK) >> IMM_WIDTH_LOW))); /* bsefi */
 else
    sprintf (p, "%d", (short)(((instr & IMM5_WIDTH_MASK) >> IMM_WIDTH_LOW) - ((instr & IMM5_MASK) >> IMM_LOW) + 1)); /* bsifi */
  return p;
}

Best regards,
Neal Frager
AMD

RE: [PATCH] microblaze: fix build error on 32-bit hosts

[Frager, Neal]

Hi Mark, Maciej, Michael,

> 
> Building with GCC-13.2.1 on Fedora 38 -- no error messages.
> 
> >Which version of GCC are you using?

> gcc (Debian 10.2.1-6) 10.2.1 20210110 on debian-i386.

> But I don't think it depends on the gcc version.  It depends on having an 32bit host that is configured with --enable-targets=all.

I added the following configs which I was not previously using in my binutils build:
--enable-targets=all
--enable-obsolete
--enable-plugins

I have also tried with various gcc versions, and I am unable to duplicate the build issue you are reporting.

The only thing I am unable to verify is using a 32-bit host.  I have only been building with a 64-bit host.

Does anyone have an idea why this patch would lead to issues with a 32-bit build host and not a 64-bit host?

Best regards,
Neal Frager
AMD

Re: [PATCH] microblaze: fix build error on 32-bit hosts

[Mark Wielaard]

Hi Neal,

On Mon, 2023-10-09 at 11:58 +0000, Frager, Neal wrote:
> The only thing I am unable to verify is using a 32-bit host.  I have only been building with a 64-bit host.
> 
> Does anyone have an idea why this patch would lead to issues with a 32-bit build host and not a 64-bit host?

It is as the error says:

elf32-microblaze.c: In function ‘microblaze_elf_relax_section’:
elf32-microblaze.c:1989:53: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka ‘unsigned int’} [-Werror=format=]
 1989 |       fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, irel->r_addend);
      |                                                   ~~^                    ~~~~~~~~~~~~~~
      |                                                     |                        |
      |                                                     long unsigned int        bfd_vma {aka unsigned int}
      |                                                   %x
elf32-microblaze.c:2074:51: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka ‘unsigned int’} [-Werror=format=]
 2074 |    fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, irelscan->r_addend);
      |                                                 ~~^                    ~~~~~~~~~~~~~~~~~~
      |                                                   |                            |
      |                                                   long unsigned int            bfd_vma {aka unsigned int}
      |                                                 %x


On a 32bit host bfd_vma is an unsigned int, but you are using a long
printf format.

Casting the r_addend to (long) is one way to resolve this.

Cheers,

Mark

P.S. If you have a commit access to binutils you can use a try branch
to test patches against builder.sourceware.org which contains several
32bit hosts. See https://sourceware.org/binutils/wiki/Buildbot

RE: [PATCH] microblaze: fix build error on 32-bit hosts

[Frager, Neal]

Hi Mark,

> The only thing I am unable to verify is using a 32-bit host.  I have only been building with a 64-bit host.
> 
> Does anyone have an idea why this patch would lead to issues with a 32-bit build host and not a 64-bit host?

> It is as the error says:

> elf32-microblaze.c: In function ‘microblaze_elf_relax_section’:
> elf32-microblaze.c:1989:53: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka ‘unsigned int’} [-Werror=format=]
> 1989 |       fprintf(stderr, "%d: CORRUPT relax reloc %x %lx\n", __LINE__, val, irel->r_addend);
>      |                                                   ~~^                    ~~~~~~~~~~~~~~
>      |                                                     |                        |
>      |                                                     long unsigned int        bfd_vma {aka unsigned int}
>      |                                                   %x
> elf32-microblaze.c:2074:51: error: format ‘%lx’ expects argument of type ‘long unsigned int’, but argument 5 has type ‘bfd_vma’ {aka ‘unsigned int’} [-Werror=format=]
> 2074 |    fprintf(stderr, "%d: CORRUPT relax reloc! %x %lx\n", __LINE__, val, irelscan->r_addend);
>      |                                                 ~~^                    ~~~~~~~~~~~~~~~~~~
>      |                                                   |                            |
>      |                                                   long unsigned int            bfd_vma {aka unsigned int}
>      |                                                 %x


> On a 32bit host bfd_vma is an unsigned int, but you are using a long printf format.

> Casting the r_addend to (long) is one way to resolve this.

I already solved this build issue.  I thought the issue Maciej was reporting was another one.

If this is the only build issue, then I will submit v3 which solves this problem now.

Could you please confirm that this is the only issue that needs to be fixed?

Best regards,
Neal Frager
AMD

Re: [PATCH] microblaze: fix build error on 32-bit hosts

[Mark Wielaard]

Hi Neal,

On Mon, 2023-10-09 at 12:42 +0000, Frager, Neal 
> > On a 32bit host bfd_vma is an unsigned int, but you are using a long printf format.
> 
> > Casting the r_addend to (long) is one way to resolve this.
> 
> I already solved this build issue.  I thought the issue Maciej was reporting was another one.
> 
> If this is the only build issue, then I will submit v3 which solves this problem now.
> 
> Could you please confirm that this is the only issue that needs to be fixed?

It is the only issue I noticed that prevented the build to complete on
a 32bit host (with --enable-target=all). I didn't try to run the
testsuite.

Cheers,

Mark

RE: [PATCH] microblaze: fix build error on 32-bit hosts

[Frager, Neal]

Hi Mark,

> > On a 32bit host bfd_vma is an unsigned int, but you are using a long printf format.
> 
> > Casting the r_addend to (long) is one way to resolve this.
> 
> I already solved this build issue.  I thought the issue Maciej was reporting was another one.
> 
> If this is the only build issue, then I will submit v3 which solves this problem now.
> 
> Could you please confirm that this is the only issue that needs to be fixed?

> It is the only issue I noticed that prevented the build to complete on a 32bit host (with --enable-target=all). I didn't try to run the testsuite.

Thank you for confirming this.  I just submitted v3 of my patch which will fix this issue.

@Maciej, could you please test v3 of my patch to verify that it resolves the issue you reported?
https://patchwork.sourceware.org/project/binutils/patch/20231009125144.377940-1-neal.frager@amd.com/

I have also added assembler tests to the gas testsuite for the new instructions.

Thank you for your help!

Best regards,
Neal Frager
AMD

RE: [PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions

[Maciej W. Rozycki]

On Mon, 9 Oct 2023, Frager, Neal wrote:

> > Add test cases for new instructions.
> > Fix GNU Coding Standard issues.
> > Run binutils test suite.
> 
> Thank you for feedback.  Could you possibly help me with resolving the 
> GNU coding standard issues and the build issue Maciej is reporting?

 See: <https://www.gnu.org/prep/standards/standards.html#Formatting> and 
<https://www.gnu.org/prep/standards/standards.html#Comments> for the 
relevant parts of the GNU Coding Standards (you're welcome to review the 
whole document).

 To reproduce the regressions configure your binutils with the command I 
provided and run the testsuite using `make check'.  Test logs will be in 
binutils/binutils.log, gas/testsuite/gas.log and ld/ld.log (there will be 
corresponding .sum files with test summaries).  There may be preexisting 
issues, but you have the list of failures to watch out for in my previous 
message.  You need to make sure no new failures have been introduced by 
your change.

 As I write I came by these issues by chance in unrelated verification 
and I won't provide further feedback as I have no interest in the 
MicroBlaze port and I cannot afford the time to be distracted.  Michael, 
the port maintainer may be able to provide you with further guidance.

 HTH,

  Maciej

Re: [PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions

[Michael Eager]

On 10/8/23 23:32, Frager, Neal wrote:
> Hi Michael, Maciej,
> 
>> Neal --
> 
>> Please resubmit this patch.
> 
>> Add test cases for new instructions.
>> Fix GNU Coding Standard issues.
>> Run binutils test suite.
> 
> Thank you for feedback.  Could you possibly help me with resolving the GNU coding standard issues and the build issue Maciej is reporting?

See other messages for GNU Coding Standards.

Maciej did not report a build issue: there are test case regressions.
See his instructions on running the binutils test suite.

> 
> Currently, the problem we are facing is that the upstream binutils is unable to build the zynqmp pmufw application.  I am trying to resolve this by upstreaming the toolchain patches AMD is currently using in its microblaze toolchain release.
> 
> This is the only patch remaining that is preventing the zynqmp pmufw application from building, as the application needs the microblaze bsefi and bsifi instructions.
> 
> My patch for adding these instructions is coming from the following two patches which AMD currently applies on its own microblaze toolchain that comes with our yocto releases:
> https://github.com/Xilinx/meta-xilinx/blob/master/meta-microblaze/recipes-devtools/binutils/binutils/0004-LOCAL-Fix-relaxation-of-assembler-resolved-reference.patch
> https://github.com/Xilinx/meta-xilinx/blob/master/meta-microblaze/recipes-devtools/binutils/binutils/0008-Add-new-bit-field-instructions.patch

I don't have time to research patches in a out-of-tree repository.  I 
did look at the other patches you submitted to see if there was some 
information about what problem the patch was solving and found nothing.
If there is relevant information in these other patches, include it with 
your patch.

> 
> As I am far away from being a toolchain or GNU binutils expert, could you please help me in the simplest of terms understand what is not following the GNU coding standard in these patches, so that I can fix it?

See other message.

> 
> My current way of testing is verifying that the master branch build of GNU binutils is able to build the zynqmp pmufw application and that it executes correctly on zynqmp hardware.  So I have verified on hardware that the v2 version of the patch I submitted is indeed working.  If you could help me with resolving these regressions, so that this patch can solve the problem I am trying to solve without causing another problem, I would appreciate any support you can provide.

That may be satisfactory for AMD's requirements, but your patch has to 
work with other targets.  I'm happy to help you, but my time is limited. 
  You (or someone else) will have to do the heavy lifting to resolve 
regressions.

For each of the test suite failures which Maciej reported, reproduce the 
problem outside of the test suite.  I believe there are error messages.

> 
> Also, are there any instructions for coding standard for generating test cases for the bsefi and bsifi instructions?  As I have never written GNU binutils test cases before, perhaps you can point me to something that will help me to save time with this as well?

To test the functions in your patch, you will need to create a small 
assembler test case which invokes the correct relocations.  The bare 
bones "is the opcode correct" test which you added will not do this.

I'll take a look at the MB test cases and see if there is one you can 
adapt.

> 
> Thank you for your support!
> 
> Best regards,
> Neal Frager
> AMD
> 
>> On Fri, 6 Oct 2023, Michael Eager wrote:
>>
>>>> This patch has been tested for years of AMD Xilinx Yocto releases as
>>>> part of the following patch set:
>>>>
>>>> https://github.com/Xilinx/meta-xilinx/tree/master/meta-microblaze/re
>>>> cipes-devtools/binutils/binutils
>>>>
>>>> Signed-off-by: nagaraju <nagaraju.mekala@amd.com>
>>>> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@amd.com>
>>>> Signed-off-by: Neal Frager <neal.frager@amd.com>
>>>
>>> Committed.
>>
>>    Yet it has caused numerous regressions:
>>
>> microblaze-elf  +FAIL: unordered .debug_info references to
>> .debug_ranges microblaze-elf  +FAIL: binutils-all/pr26548
>> microblaze-elf  +FAIL: readelf -Wwi pr26548e (reason: unexpected
>> output) microblaze-elf  +FAIL: readelf --debug-dump=loc locview-1
>> (reason: unexpected output) microblaze-elf  +FAIL: readelf
>> --debug-dump=loc locview-2 (reason: unexpected output) microblaze-elf
>> +FAIL: readelf -wiaoRlL dw5 microblaze-elf  +FAIL: readelf -wi
>> dwarf-attributes (reason: unexpected output) microblaze-elf  +FAIL:
>> readelf -wKis -P debuglink (reason: unexpected output) microblaze-elf
>> +FAIL: readelf --debug-dump=links --debug-dump=no-follow-links dwo
>> microblaze-elf  +FAIL: DWARF2 1 microblaze-elf  +FAIL: DWARF2 2
>> microblaze-elf  +FAIL: DWARF2 3 microblaze-elf  +FAIL: DWARF2 5
>> microblaze-elf  +FAIL: DWARF2 6 microblaze-elf  +FAIL: DWARF2 7
>> microblaze-elf  +FAIL: DWARF2 11 microblaze-elf  +FAIL: DWARF2 12
>> microblaze-elf  +FAIL: DWARF2 13 microblaze-elf  +FAIL: DWARF2 14
>> microblaze-elf  +FAIL: DWARF2 15 microblaze-elf  +FAIL: DWARF2 16
>> microblaze-elf  +FAIL: DWARF2 17 microblaze-elf  +FAIL: DWARF2 18
>> microblaze-elf  +FAIL: DWARF2 19 microblaze-elf  +FAIL: DWARF2_20:
>> debug ranges ignore non-code sections microblaze-elf  +FAIL: DWARF2 21
>> microblaze-elf  +FAIL: DWARF5 .loc 0 microblaze-elf  +FAIL: DWARF4 CU
>> microblaze-elf  +FAIL: DWARF5 CU microblaze-elf  +FAIL: Check line
>> table is produced with .nops microblaze-elf  +FAIL: line number
>> entries for section changes inside .irp microblaze-elf  +FAIL: line
>> number entries for .macro expansions microblaze-elf  +FAIL: line
>> number entries for expansions of .macro coming from .include
>> microblaze-elf  +FAIL: lns-duplicate microblaze-elf  +FAIL:
>> lns-common-1 microblaze-elf  +FAIL: ld-elf/pr22450
>>
>> They all seem to follow a similar pattern, e.g:
>>
>> exited abnormally with 1, output:readelf: Warning: Corrupt unit length
>> (got 0x4e00004e expected at most 0x4e) in section .debug_info
>>
>> FAIL: DWARF2 1
>>
>> or:
>>
>> exited abnormally with 0, output:readelf: Warning: Corrupt unit length
>> (got 0x20000020 expected at most 0x20) in section .debug_info
>>
>> FAIL: Check line table is produced with .nops
>>
>> Configured with:
>>
>> $ /path/to/configure --disable-nls --disable-gdb --disable-gdbserver
>> --disable-gprofng --disable-libbacktrace --disable-libdecnumber
>> --disable-readline --disable-sim --enable-obsolete --enable-plugins
>> --build=powerpc64le-linux --target=microblaze-elf
>>
>>    Just reporting in case it's useful as it has popped up in unrelated
>> verification; I won't do anything else here.
>>
>>    NB I've skimmed over the change and noticed it does not follow the
>> GNU Coding Standards in many places, most prominently in `get_field_imm5width'
>> and `get_field_rfsl', but also elsewhere (which is easy to spot
>> looking through the diff).
>>
>>     Maciej
>>
> 
>> --
>> Michael Eager

Re: [PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions

[Michael Eager]

On 10/9/23 03:53, Frager, Neal wrote:
> 
>>> This patch has been tested for years of AMD Xilinx Yocto releases as
>>> part of the following patch set:
>>>
>>> https://github.com/Xilinx/meta-xilinx/tree/master/meta-microblaze/re
>>> cipes-devtools/binutils/binutils
>>>
>>> Signed-off-by: nagaraju <nagaraju.mekala@amd.com>
>>> Signed-off-by: Ibai Erkiaga <ibai.erkiaga-elorza@amd.com>
>>> Signed-off-by: Neal Frager <neal.frager@amd.com>
>>
>> Committed.
> 
>> Yet it has caused numerous regressions:
> 
>> microblaze-elf  +FAIL: unordered .debug_info references to .debug_ranges microblaze-elf  +FAIL: binutils-all/pr26548 microblaze-elf  +FAIL: readelf -Wwi pr26548e (reason: unexpected output) microblaze-elf  +FAIL: readelf --debug-dump=loc locview-1 (reason: unexpected output) microblaze-elf  +FAIL: readelf --debug-dump=loc locview-2 (reason: unexpected output) microblaze-elf  +FAIL: readelf -wiaoRlL dw5 microblaze-elf  +FAIL: readelf -wi dwarf-attributes (reason: unexpected output) microblaze-elf  +FAIL: readelf -wKis -P debuglink (reason: unexpected output) microblaze-elf  +FAIL: readelf --debug-dump=links --debug-dump=no-follow-links dwo microblaze-elf  +FAIL: DWARF2 1 microblaze-elf  +FAIL: DWARF2 2 microblaze-elf  +FAIL: DWARF2 3 microblaze-elf  +FAIL: DWARF2 5 microblaze-elf  +FAIL: DWARF2 6 microblaze-elf  +FAIL: DWARF2 7 microblaze-elf  +FAIL: DWARF2 11 microblaze-elf  +FAIL: DWARF2 12 microblaze-elf  +FAIL: DWARF2 13 microblaze-elf  +FAIL: DWARF2 14 microblaze-elf  +FAIL: DWARF2 15 microblaze-elf  +FAIL: DWARF2 16 microblaze-elf  +FAIL: DWARF2 17 microblaze-elf  +FAIL: DWARF2 18 microblaze-elf  +FAIL: DWARF2 19 microblaze-elf  +FAIL: DWARF2_20: debug ranges ignore non-code sections microblaze-elf  +FAIL: DWARF2 21 microblaze-elf  +FAIL: DWARF5 .loc 0 microblaze-elf  +FAIL: DWARF4 CU microblaze-elf  +FAIL: DWARF5 CU microblaze-elf  +FAIL: Check line table is produced with .nops microblaze-elf  +FAIL: line number entries for section changes inside .irp microblaze-elf  +FAIL: line number entries for .macro expansions microblaze-elf  +FAIL: line number entries for expansions of .macro coming from .include microblaze-elf  +FAIL: lns-duplicate microblaze-elf  +FAIL: lns-common-1 microblaze-elf  +FAIL: ld-elf/pr22450
> 
>> They all seem to follow a similar pattern, e.g:
> 
>> exited abnormally with 1, output:readelf: Warning: Corrupt unit length (got 0x4e00004e expected at most 0x4e) in section .debug_info
> 
>> FAIL: DWARF2 1
> 
>> or:
> 
>> exited abnormally with 0, output:readelf: Warning: Corrupt unit length (got 0x20000020 expected at most 0x20) in section .debug_info
> 
>> FAIL: Check line table is produced with .nops
> 
>> Configured with:
> 
>> $ /path/to/configure --disable-nls --disable-gdb --disable-gdbserver --disable-gprofng --disable-libbacktrace --disable-libdecnumber --disable-readline --disable-sim --enable-obsolete --enable-plugins --build=powerpc64le-linux --target=microblaze-elf
> 
>> Just reporting in case it's useful as it has popped up in unrelated verification; I won't do anything else here.
> 
>> NB I've skimmed over the change and noticed it does not follow the GNU Coding Standards in many places, most prominently in `get_field_imm5width'
> and `get_field_rfsl', but also elsewhere (which is easy to spot looking through the diff).
> 
> Hi Maciej,
> 
> The patch I submitted does not modify the 'get_field_rfsl' function, but perhaps you could review
> the 'get_field_imm5width' function?  Could you explain what is not following the GNU Coding
> Standards, so that I can fix it?
> 
> static char *
> get_field_imm5width (struct string_buf *buf, long instr)
> {
>    char *p = strbuf (buf);
> 
>    if (instr & 0x00004000)
>      sprintf (p, "%d", (short)(((instr & IMM5_WIDTH_MASK) >> IMM_WIDTH_LOW))); /* bsefi */
>   else
>      sprintf (p, "%d", (short)(((instr & IMM5_WIDTH_MASK) >> IMM_WIDTH_LOW) - ((instr & IMM5_MASK) >> IMM_LOW) + 1)); /* bsifi */
>    return p;
> }

Lines should be shorter than 80 characters.

> 

-- 
Michael Eager

RE: [PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions

[Frager, Neal]

> Hi Michael, Maciej,
> 
>> Neal --
> 
>> Please resubmit this patch.
> 
>> Add test cases for new instructions.
>> Fix GNU Coding Standard issues.
>> Run binutils test suite.
> 
> Thank you for feedback.  Could you possibly help me with resolving the GNU coding standard issues and the build issue Maciej is reporting?

> See other messages for GNU Coding Standards.

> Maciej did not report a build issue: there are test case regressions.
> See his instructions on running the binutils test suite.

> 
> Currently, the problem we are facing is that the upstream binutils is unable to build the zynqmp pmufw application.  I am trying to resolve this by upstreaming the toolchain patches AMD is currently using in its microblaze toolchain release.
> 
> This is the only patch remaining that is preventing the zynqmp pmufw application from building, as the application needs the microblaze bsefi and bsifi instructions.
> 
> My patch for adding these instructions is coming from the following two patches which AMD currently applies on its own microblaze toolchain that comes with our yocto releases:
> https://github.com/Xilinx/meta-xilinx/blob/master/meta-microblaze/reci
> pes-devtools/binutils/binutils/0004-LOCAL-Fix-relaxation-of-assembler-
> resolved-reference.patch 
> https://github.com/Xilinx/meta-xilinx/blob/master/meta-microblaze/reci
> pes-devtools/binutils/binutils/0008-Add-new-bit-field-instructions.pat
> ch

> I don't have time to research patches in a out-of-tree repository.  I did look at the other patches you submitted to see if there was some information about what problem the patch was solving and found nothing.
> If there is relevant information in these other patches, include it with your patch.

Hi Michael,

You should not need to look at these patches.  Everything implemented by these patches has been included in the patch I submitted.  I only mentioned them to show where the patch I submitted originated from.

> 
> As I am far away from being a toolchain or GNU binutils expert, could you please help me in the simplest of terms understand what is not following the GNU coding standard in these patches, so that I can fix it?

> See other message.

I just submitted v4 of the patch with the following updates:
  - Cleaned up all of the GNU coding standard issues.
  - Removed any line modifications which were unnecessary, so v4 of the patch looks much cleaner now.
  - Tested this patch on the AMD functional side in that it not only builds, but the built binutils are able to build the zynqmp pmufw application correctly.
  - Verified that the 32-bit host machine build issue is resolved.

As far as I can tell, the only thing missing is a test case for the gas testsuite to verify the relocation works as expected.

Would you be ok with applying the current v4 of the patch while I work on this last part?

> 
> My current way of testing is verifying that the master branch build of GNU binutils is able to build the zynqmp pmufw application and that it executes correctly on zynqmp hardware.  So I have verified on hardware that the v2 version of the patch I submitted is indeed working.  If you could help me with resolving these regressions, so that this patch can solve the problem I am trying to solve without causing another problem, I would appreciate any support you can provide.

> That may be satisfactory for AMD's requirements, but your patch has to 
> work with other targets.  I'm happy to help you, but my time is limited. 
>  You (or someone else) will have to do the heavy lifting to resolve 
> regressions.

> For each of the test suite failures which Maciej reported, reproduce the 
> problem outside of the test suite.  I believe there are error messages.

Are you sure that the problems Maciej reported can be reproduced outside of the test suite?  If so, how?

> 
> Also, are there any instructions for coding standard for generating test cases for the bsefi and bsifi instructions?  As I have never written GNU binutils test cases before, perhaps you can point me to something that will help me to save time with this as well?

> To test the functions in your patch, you will need to create a small 
> assembler test case which invokes the correct relocations.  The bare 
> bones "is the opcode correct" test which you added will not do this.

> I'll take a look at the MB test cases and see if there is one you can 
> adapt.

Understood and thank you for your help.  I will work on this now.
 
Thank you for your support!
 
Best regards,
Neal Frager
AMD

Re: [PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions

[Michael Eager]

On 10/9/23 23:54, Frager, Neal wrote:
> 
> Hi Michael,
> 

> I just submitted v4 of the patch with the following updates:
>    - Cleaned up all of the GNU coding standard issues.
>    - Removed any line modifications which were unnecessary, so v4 of the patch looks much cleaner now.
>    - Tested this patch on the AMD functional side in that it not only builds, but the built binutils are able to build the zynqmp pmufw application correctly.
>    - Verified that the 32-bit host machine build issue is resolved.

Being able to build and execute in your environment is not adequate.
You need to verify that it does not break other targets.

> As far as I can tell, the only thing missing is a test case for the gas testsuite to verify the relocation works as expected.
> 
> Would you be ok with applying the current v4 of the patch while I work on this last part?

The test suite needs to run cleanly, without regressions, before the 
patch can be applied.

A test case to confirm that the relocation works would be good, but is 
not a requirement.

>> My current way of testing is verifying that the master branch build of GNU binutils is able to build the zynqmp pmufw application and that it executes correctly on zynqmp hardware.  So I have verified on hardware that the v2 version of the patch I submitted is indeed working.  If you could help me with resolving these regressions, so that this patch can solve the problem I am trying to solve without causing another problem, I would appreciate any support you can provide.
> 
>> That may be satisfactory for AMD's requirements, but your patch has to
>> work with other targets.  I'm happy to help you, but my time is limited.
>>   You (or someone else) will have to do the heavy lifting to resolve
>> regressions.
> 
>> For each of the test suite failures which Maciej reported, reproduce the
>> problem outside of the test suite.  I believe there are error messages.
> 
> Are you sure that the problems Maciej reported can be reproduced outside of the test suite?  If so, how?

Build binutils using the options that Maciej used.  Do not apply your 
latest patch.

Run the test suite ("make check").

Look at testsuite/gas.log.  Save it.

Apply the patch, run make, run make check.

Look at testsuite/gas.log.  Diff with saved version.

In the log, there are results of running each test, indicating failures.
You can run these same commands (as-new, readelf) from the command line.

FAIL: DWARF2 1
../as-new  --compress-debug-sections  -o tmpdir/dwarf2-2.o 
binutils/gas/testsuite/gas/elf/dwarf2-2.s
Executing on host: sh -c {../as-new  --compress-debug-sections  -o 
tmpdir/dwarf2-2.o binutils/gas/testsuite/gas/elf/dwarf2-2.s 2>&1} 
/dev/null dump.tmp (timeout = 300)
spawn [open ...]
build/gas/testsuite/../../binutils/readelf  -w tmpdir/dwarf2-2.o > 
tmpdir/dump.out
Executing on host: sh -c {build/gas/testsuite/../../binutils/readelf  -w 
tmpdir/dwarf2-2.o > tmpdir/dump.out 2>dump.tmp}  /dev/null  (timeout = 300)
spawn [open ...]
exited abnormally with 1, output:readelf: Warning: Corrupt unit length 
(got 0x4e00004e expected at most 0x4e) in section .debug_info

>> Also, are there any instructions for coding standard for generating test cases for the bsefi and bsifi instructions?  As I have never written GNU binutils test cases before, perhaps you can point me to something that will help me to save time with this as well?
> 
>> To test the functions in your patch, you will need to create a small
>> assembler test case which invokes the correct relocations.  The bare
>> bones "is the opcode correct" test which you added will not do this.
> 
>> I'll take a look at the MB test cases and see if there is one you can
>> adapt.

Look at binutils/gas/testsuite/gas/microblaze/reloc_sym.{s,d}.


> 
> Understood and thank you for your help.  I will work on this now.
>   
> Thank you for your support!
>   
> Best regards,
> Neal Frager
> AMD

-- 
Michael Eager

Re: [PATCH v2 1/1] opcodes: microblaze: Add new bit-field instructions

[Michael Eager]

On 10/10/23 09:16, Michael Eager wrote:
> On 10/9/23 23:54, Frager, Neal wrote:
>>
>> Hi Michael,
>>
> 
>> I just submitted v4 of the patch with the following updates:
>>    - Cleaned up all of the GNU coding standard issues.
>>    - Removed any line modifications which were unnecessary, so v4 of 
>> the patch looks much cleaner now.
>>    - Tested this patch on the AMD functional side in that it not only 
>> builds, but the built binutils are able to build the zynqmp pmufw 
>> application correctly.
>>    - Verified that the 32-bit host machine build issue is resolved.
> 
> Being able to build and execute in your environment is not adequate.
> You need to verify that it does not break other targets.

To be clear:  the test suite regressions are in the MicroBlaze 
assembler, not in other targets.  These need to be resolved.

> 
>> As far as I can tell, the only thing missing is a test case for the 
>> gas testsuite to verify the relocation works as expected.
>>
>> Would you be ok with applying the current v4 of the patch while I work 
>> on this last part?
> 
> The test suite needs to run cleanly, without regressions, before the 
> patch can be applied.
> 
> A test case to confirm that the relocation works would be good, but is 
> not a requirement.
> 
>>> My current way of testing is verifying that the master branch build 
>>> of GNU binutils is able to build the zynqmp pmufw application and 
>>> that it executes correctly on zynqmp hardware.  So I have verified on 
>>> hardware that the v2 version of the patch I submitted is indeed 
>>> working.  If you could help me with resolving these regressions, so 
>>> that this patch can solve the problem I am trying to solve without 
>>> causing another problem, I would appreciate any support you can provide.
>>
>>> That may be satisfactory for AMD's requirements, but your patch has to
>>> work with other targets.  I'm happy to help you, but my time is limited.
>>>   You (or someone else) will have to do the heavy lifting to resolve
>>> regressions.
>>
>>> For each of the test suite failures which Maciej reported, reproduce the
>>> problem outside of the test suite.  I believe there are error messages.
>>
>> Are you sure that the problems Maciej reported can be reproduced 
>> outside of the test suite?  If so, how?
> 
> Build binutils using the options that Maciej used.  Do not apply your 
> latest patch.
> 
> Run the test suite ("make check").
> 
> Look at testsuite/gas.log.  Save it.
> 
> Apply the patch, run make, run make check.
> 
> Look at testsuite/gas.log.  Diff with saved version.
> 
> In the log, there are results of running each test, indicating failures.
> You can run these same commands (as-new, readelf) from the command line.
> 
> FAIL: DWARF2 1
> ../as-new  --compress-debug-sections  -o tmpdir/dwarf2-2.o 
> binutils/gas/testsuite/gas/elf/dwarf2-2.s
> Executing on host: sh -c {../as-new  --compress-debug-sections  -o 
> tmpdir/dwarf2-2.o binutils/gas/testsuite/gas/elf/dwarf2-2.s 2>&1} 
> /dev/null dump.tmp (timeout = 300)
> spawn [open ...]
> build/gas/testsuite/../../binutils/readelf  -w tmpdir/dwarf2-2.o > 
> tmpdir/dump.out
> Executing on host: sh -c {build/gas/testsuite/../../binutils/readelf  -w 
> tmpdir/dwarf2-2.o > tmpdir/dump.out 2>dump.tmp}  /dev/null  (timeout = 300)
> spawn [open ...]
> exited abnormally with 1, output:readelf: Warning: Corrupt unit length 
> (got 0x4e00004e expected at most 0x4e) in section .debug_info
> 
>>> Also, are there any instructions for coding standard for generating 
>>> test cases for the bsefi and bsifi instructions?  As I have never 
>>> written GNU binutils test cases before, perhaps you can point me to 
>>> something that will help me to save time with this as well?
>>
>>> To test the functions in your patch, you will need to create a small
>>> assembler test case which invokes the correct relocations.  The bare
>>> bones "is the opcode correct" test which you added will not do this.
>>
>>> I'll take a look at the MB test cases and see if there is one you can
>>> adapt.
> 
> Look at binutils/gas/testsuite/gas/microblaze/reloc_sym.{s,d}.
> 
> 
>>
>> Understood and thank you for your help.  I will work on this now.
>> Thank you for your support!
>> Best regards,
>> Neal Frager
>> AMD
> 

-- 
Michael Eager