[v2] microblaze: add BFD_RELOC_8 and BFD_RELOC_16 cases

  From: Samuel Price <thesamprice@gmail.com>

md_apply_fix has cases for BFD_RELOC_32, BFD_RELOC_64 and for the
MicroBlaze-specific relocations, but none for BFD_RELOC_8 or BFD_RELOC_16.
Those fall through to the default and nothing is written, so a byte or
halfword datum whose value is only known at fixup time assembles to zero.
The fixup is then resolved and turned into BFD_RELOC_NONE, so no relocation
survives for the linker to correct it either, and the zero is permanent.

The two new cases replicate the BFD_RELOC_32 case immediately below them in
the same switch -- the same S_IS_DEFINED guard and the same target_big_endian
byte order -- narrowed to one and two bytes.

Fixes gas/all/simple-forward.  It also fixes gas/all/forward, which was
xfailed for microblaze-*-*, so drop it from that xfail list.  The gas
testsuite on microblaze-elf goes from 327 passes and 1 unexpected failure to
329 passes and 0 failures.

gas/
	* config/tc-microblaze.c (md_apply_fix): Handle BFD_RELOC_8 and
	BFD_RELOC_16.
	* testsuite/gas/all/forward.d: Remove the microblaze-*-* xfail.

Signed-off-by: Sam Price <thesamprice@gmail.com>
Reviewed-by: Neal Frager <neal.frager@amd.com>
Assisted-by: Claude (Anthropic)
---

Changes since v1:

  - forward.d: also update the comment above the #xfail line, which still
    read "mep and microblaze use complex relocs and don't resolve the
    relocs" after microblaze was dropped from the xfail list.  Caught by
    Neal Frager.  Reworded for the now-single target: "mep uses ...
    doesn't".

No change to gas/config/tc-microblaze.c.  The testsuite result is unchanged
and was re-measured on f621e384d9c with the new comment in place: 327 passes
and 1 unexpected failure -> 329 passes and 0, no unexpected successes.  The
comment is inert to DejaGnu.

v1: https://inbox.sourceware.org/binutils/20260920235724.37190-1-thesamprice@gmail.com/

I considered writing the two new cases the way aarch64 and mips do instead --

  case BFD_RELOC_16:
  case BFD_RELOC_8:
    if (fixP->fx_done)
      md_number_to_chars (buf, *valP, fixP->fx_size);
    break;

-- which is shorter, drops the endianness branch, and guards on fx_done
rather than S_IS_DEFINED, which is arguably what is really meant here.  The
comment on that case in tc-mips.c describes this exact failure mode: "If we
are deleting this reloc entry, we must fill in the value now."

I kept the local form so the patch stays minimal and consistent with its
neighbour, but I am happy to send the md_number_to_chars version instead if
you would rather have it.  Or if you want the neighbors converted to
aarch64 / mips and tested I can do that also.

 gas/config/tc-microblaze.c      | 23 +++++++++++++++++++++++
 gas/testsuite/gas/all/forward.d |  4 ++--
 2 files changed, 25 insertions(+), 2 deletions(-)
  

On 9/21/26 6:29 PM, Sam Price wrote:
> From: Samuel Price <thesamprice@gmail.com>
> 
> md_apply_fix has cases for BFD_RELOC_32, BFD_RELOC_64 and for the
> MicroBlaze-specific relocations, but none for BFD_RELOC_8 or BFD_RELOC_16.
> Those fall through to the default and nothing is written, so a byte or
> halfword datum whose value is only known at fixup time assembles to zero.
> The fixup is then resolved and turned into BFD_RELOC_NONE, so no relocation
> survives for the linker to correct it either, and the zero is permanent.
> 
> The two new cases replicate the BFD_RELOC_32 case immediately below them in
> the same switch -- the same S_IS_DEFINED guard and the same target_big_endian
> byte order -- narrowed to one and two bytes.
> 
> Fixes gas/all/simple-forward.  It also fixes gas/all/forward, which was
> xfailed for microblaze-*-*, so drop it from that xfail list.  The gas
> testsuite on microblaze-elf goes from 327 passes and 1 unexpected failure to
> 329 passes and 0 failures.
> 
> gas/
> 	* config/tc-microblaze.c (md_apply_fix): Handle BFD_RELOC_8 and
> 	BFD_RELOC_16.
> 	* testsuite/gas/all/forward.d: Remove the microblaze-*-* xfail.
> 
> Signed-off-by: Sam Price <thesamprice@gmail.com>
> Reviewed-by: Neal Frager <neal.frager@amd.com>
> Assisted-by: Claude (Anthropic)
> ---
> 
> Changes since v1:
> 
>    - forward.d: also update the comment above the #xfail line, which still
>      read "mep and microblaze use complex relocs and don't resolve the
>      relocs" after microblaze was dropped from the xfail list.  Caught by
>      Neal Frager.  Reworded for the now-single target: "mep uses ...
>      doesn't".
> 
> No change to gas/config/tc-microblaze.c.  The testsuite result is unchanged
> and was re-measured on f621e384d9c with the new comment in place: 327 passes
> and 1 unexpected failure -> 329 passes and 0, no unexpected successes.  The
> comment is inert to DejaGnu.
> 
> v1: https://inbox.sourceware.org/binutils/20260920235724.37190-1-thesamprice@gmail.com/
> 
> I considered writing the two new cases the way aarch64 and mips do instead --
> 
>    case BFD_RELOC_16:
>    case BFD_RELOC_8:
>      if (fixP->fx_done)
>        md_number_to_chars (buf, *valP, fixP->fx_size);
>      break;
> 
> -- which is shorter, drops the endianness branch, and guards on fx_done
> rather than S_IS_DEFINED, which is arguably what is really meant here.  The
> comment on that case in tc-mips.c describes this exact failure mode: "If we
> are deleting this reloc entry, we must fill in the value now."
> 
> I kept the local form so the patch stays minimal and consistent with its
> neighbour, but I am happy to send the md_number_to_chars version instead if
> you would rather have it.  Or if you want the neighbors converted to
> aarch64 / mips and tested I can do that also.

I think that using md_number_to_chars() like aarch64, mips, sparc, 
others is better.

Cleaning up the similar code for BFD_RELOC_MICROBLAZE_... would also be 
a good thing.

> 
>   gas/config/tc-microblaze.c      | 23 +++++++++++++++++++++++
>   gas/testsuite/gas/all/forward.d |  4 ++--
>   2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/gas/config/tc-microblaze.c b/gas/config/tc-microblaze.c
> index 3e3dca81921..36b98bbd8fc 100644
> --- a/gas/config/tc-microblaze.c
> +++ b/gas/config/tc-microblaze.c
> @@ -2145,6 +2145,29 @@ md_apply_fix (fixS *   fixP,
>   	    }
>   	}
>         break;
> +    case BFD_RELOC_8:
> +      /* Don't do anything if the symbol is not defined.  */
> +      if (fixP->fx_addsy == NULL || S_IS_DEFINED (fixP->fx_addsy))
> +	buf[0] |= (val & 0xff);
> +      break;
> +
> +    case BFD_RELOC_16:
> +      /* Don't do anything if the symbol is not defined.  */
> +      if (fixP->fx_addsy == NULL || S_IS_DEFINED (fixP->fx_addsy))
> +	{
> +	  if (target_big_endian)
> +	    {
> +	      buf[0] |= ((val >> 8) & 0xff);
> +	      buf[1] |= (val & 0xff);
> +	    }
> +	  else
> +	    {
> +	      buf[1] |= ((val >> 8) & 0xff);
> +	      buf[0] |= (val & 0xff);
> +	    }
> +	}
> +      break;
> +
>       case BFD_RELOC_32:
>       case BFD_RELOC_RVA:
>       case BFD_RELOC_32_PCREL:
> diff --git a/gas/testsuite/gas/all/forward.d b/gas/testsuite/gas/all/forward.d
> index d921ec517be..39f7b2941e8 100644
> --- a/gas/testsuite/gas/all/forward.d
> +++ b/gas/testsuite/gas/all/forward.d
> @@ -8,10 +8,10 @@
>   # choose to emit them even though the relocs are in .data, leading to
>   # "redefined symbol cannot be used on reloc".
>   #xfail: am33*-*-* crx*-*-* h8300*-*-* mn10200*-*-* mn10300*-*-*
> -# mep and microblaze use complex relocs and don't resolve the relocs.
> +# mep uses complex relocs and doesn't resolve the relocs.
>   # one of the relocs references minus_one, which is a bug, but no one
>   # seems bothered enough to fix this.
> -#xfail: mep-*-* microblaze-*-*
> +#xfail: mep-*-*
>   
>   .*: .*
>
  

On 9/23/26 6:44 PM, Sam Price wrote:
> From: Samuel Price <thesamprice@gmail.com>
> 
> 1/4 is the v2 patch with review applied: the two new cases
> write through md_number_to_chars, as aarch64, mips and sparc do, instead of
> open-coding the bytes.
> 
> 2/4 is the cleanup he asked for in the same reply -- the rest of md_apply_fix
> converted the same way.
> 
> 4/4 finishes the job.  The INST_BYTE0..3 macros expand to exactly a
> target-order four-byte store, so the six places in md_assemble that spell one
> out become single calls, and the macros themselves go away.
> 
> 3/4 is testsuite only.  Testing on microblazeel-elf as well as microblaze-elf
> turned up two gas/all failures that are nothing to do with this series:
> gas.exp excludes MicroBlaze from the diff1 and end tests, but both guards say
> "microblaze-*-*", which does not match the microblazel triplet.  Widening
> them is a two-character fix and it takes microblazeel to zero unexpected
> failures, so it seemed better to send it here than on its own.
> 
> If I should squash these all into one commit let me know, or do that in the
> future.
> 
> Net effect on tc-microblaze.c: 26 insertions, 80 deletions.
> 
> Testing -- whole testsuite, both byte orders, as expected passes /
> unexpected failures:
> 
>                         gas        binutils      ld
>    microblaze-elf       329/0      240/0         480/0
>    microblazeel-elf     328/0      241/0         480/0
> 
> No unexpected successes anywhere.  Every column is a measured before/after
> against pristine master, not a single-sided run:
> 
>    - gas moves as intended: microblaze-elf 327/1 -> 329/0, microblazeel-elf
>      326/4 -> 328/0.
> 
>    - binutils and ld come out identical test by test, not merely in their
>      totals -- 1473 results compared across the two byte orders, no
>      differences.  That is the evidence for 2/4 and 4/4 being no functional
>      change.
> 
> ld matters here in particular, since that suite exercises relaxation, which
> is where a wrong immediate offset would show up rather than in gas.  4/4
> touches md_assemble, which runs for every instruction rather than only for
> relocations, which is why the whole set was re-run rather than just gas.
> 
> Changes since v2:
> 
>    - 1/4: use md_number_to_chars (Michael Eager).  The target_big_endian
>      branch goes away; these are data relocations at offset 0.  The guard
>      changes with it, from S_IS_DEFINED (fixP->fx_addsy) to fixP->fx_done,
>      matching tc-mips.c.  That is a behaviour change rather than a
>      reformatting, so flagging it explicitly: fx_done is the condition that
>      actually means no relocation will survive for the linker.
> 
>    - 2/4, 3/4 and 4/4: new in v3.
> 
>    - Dropped Neal Frager's Reviewed-by from v2.  He reviewed the open-coded
>      form and 1/4 is now a different implementation, so the tag is his to
>      re-offer.
> 
> One thing 2/4 does not do: the endianness test does not disappear for the
> instruction relocations, only for the data ones.  A 32-bit instruction word
> is stored in target byte order, so its immediate field moves as well as its
> bytes -- offset 2 big-endian, offset 0 little-endian.  md_number_to_chars
> fixes the order but not the offset, so that residue stays, named
> INST_IMM_OFFSET rather than left as a bare ternary.  It is the only
> target_big_endian test left in the file after 4/4.
> 
> v1: https://inbox.sourceware.org/binutils/20260920235724.37190-1-thesamprice@gmail.com/
> v2: https://inbox.sourceware.org/binutils/20260922012957.86519-1-thesamprice@gmail.com/
> 
> Samuel Price (4):
>    microblaze: add BFD_RELOC_8 and BFD_RELOC_16 cases
>    microblaze: use md_number_to_chars in md_apply_fix
>    gas: testsuite: exclude microblazeel from the diff1 and end tests
>    microblaze: use md_number_to_chars in md_assemble too
> 
> Samuel Price (4):
>    microblaze: add BFD_RELOC_8 and BFD_RELOC_16 cases
>    microblaze: use md_number_to_chars in md_apply_fix
>    gas: testsuite: exclude microblazeel from the diff1 and end tests
>    microblaze: use md_number_to_chars in md_assemble too
> 
>   gas/config/tc-microblaze.c      | 108 +++++++-------------------------
>   gas/testsuite/gas/all/forward.d |   4 +-
>   gas/testsuite/gas/all/gas.exp   |   4 +-
>   3 files changed, 28 insertions(+), 88 deletions(-)
> 

Nicely done.  Splitting into four smaller patches made review much 
easier.  Thanks for describing the test results.

Committed:
   1/4:  ee6b7f5bb...
   2/4:  5047dd4a3...
   3/4:  ad8ac4b89...
   4/4:  0dabe1eeb...
  

Thanks,
I am still learning the upstream process.  I wasn't sure if you wanted them
split like this or just a giant patch.
I will go for smaller patch chains for easier reviews in the future.

Thanks,
Sam

On Thu, Sep 24, 2026 at 11:38 AM Michael Eager <eager@eagerm.com> wrote:

> On 9/23/26 6:44 PM, Sam Price wrote:
> > From: Samuel Price <thesamprice@gmail.com>
> >
> > 1/4 is the v2 patch with review applied: the two new cases
> > write through md_number_to_chars, as aarch64, mips and sparc do, instead
> of
> > open-coding the bytes.
> >
> > 2/4 is the cleanup he asked for in the same reply -- the rest of
> md_apply_fix
> > converted the same way.
> >
> > 4/4 finishes the job.  The INST_BYTE0..3 macros expand to exactly a
> > target-order four-byte store, so the six places in md_assemble that
> spell one
> > out become single calls, and the macros themselves go away.
> >
> > 3/4 is testsuite only.  Testing on microblazeel-elf as well as
> microblaze-elf
> > turned up two gas/all failures that are nothing to do with this series:
> > gas.exp excludes MicroBlaze from the diff1 and end tests, but both
> guards say
> > "microblaze-*-*", which does not match the microblazel triplet.  Widening
> > them is a two-character fix and it takes microblazeel to zero unexpected
> > failures, so it seemed better to send it here than on its own.
> >
> > If I should squash these all into one commit let me know, or do that in
> the
> > future.
> >
> > Net effect on tc-microblaze.c: 26 insertions, 80 deletions.
> >
> > Testing -- whole testsuite, both byte orders, as expected passes /
> > unexpected failures:
> >
> >                         gas        binutils      ld
> >    microblaze-elf       329/0      240/0         480/0
> >    microblazeel-elf     328/0      241/0         480/0
> >
> > No unexpected successes anywhere.  Every column is a measured
> before/after
> > against pristine master, not a single-sided run:
> >
> >    - gas moves as intended: microblaze-elf 327/1 -> 329/0,
> microblazeel-elf
> >      326/4 -> 328/0.
> >
> >    - binutils and ld come out identical test by test, not merely in their
> >      totals -- 1473 results compared across the two byte orders, no
> >      differences.  That is the evidence for 2/4 and 4/4 being no
> functional
> >      change.
> >
> > ld matters here in particular, since that suite exercises relaxation,
> which
> > is where a wrong immediate offset would show up rather than in gas.  4/4
> > touches md_assemble, which runs for every instruction rather than only
> for
> > relocations, which is why the whole set was re-run rather than just gas.
> >
> > Changes since v2:
> >
> >    - 1/4: use md_number_to_chars (Michael Eager).  The target_big_endian
> >      branch goes away; these are data relocations at offset 0.  The guard
> >      changes with it, from S_IS_DEFINED (fixP->fx_addsy) to
> fixP->fx_done,
> >      matching tc-mips.c.  That is a behaviour change rather than a
> >      reformatting, so flagging it explicitly: fx_done is the condition
> that
> >      actually means no relocation will survive for the linker.
> >
> >    - 2/4, 3/4 and 4/4: new in v3.
> >
> >    - Dropped Neal Frager's Reviewed-by from v2.  He reviewed the
> open-coded
> >      form and 1/4 is now a different implementation, so the tag is his to
> >      re-offer.
> >
> > One thing 2/4 does not do: the endianness test does not disappear for the
> > instruction relocations, only for the data ones.  A 32-bit instruction
> word
> > is stored in target byte order, so its immediate field moves as well as
> its
> > bytes -- offset 2 big-endian, offset 0 little-endian.  md_number_to_chars
> > fixes the order but not the offset, so that residue stays, named
> > INST_IMM_OFFSET rather than left as a bare ternary.  It is the only
> > target_big_endian test left in the file after 4/4.
> >
> > v1:
> https://inbox.sourceware.org/binutils/20260920235724.37190-1-thesamprice@gmail.com/
> > v2:
> https://inbox.sourceware.org/binutils/20260922012957.86519-1-thesamprice@gmail.com/
> >
> > Samuel Price (4):
> >    microblaze: add BFD_RELOC_8 and BFD_RELOC_16 cases
> >    microblaze: use md_number_to_chars in md_apply_fix
> >    gas: testsuite: exclude microblazeel from the diff1 and end tests
> >    microblaze: use md_number_to_chars in md_assemble too
> >
> > Samuel Price (4):
> >    microblaze: add BFD_RELOC_8 and BFD_RELOC_16 cases
> >    microblaze: use md_number_to_chars in md_apply_fix
> >    gas: testsuite: exclude microblazeel from the diff1 and end tests
> >    microblaze: use md_number_to_chars in md_assemble too
> >
> >   gas/config/tc-microblaze.c      | 108 +++++++-------------------------
> >   gas/testsuite/gas/all/forward.d |   4 +-
> >   gas/testsuite/gas/all/gas.exp   |   4 +-
> >   3 files changed, 28 insertions(+), 88 deletions(-)
> >
>
> Nicely done.  Splitting into four smaller patches made review much
> easier.  Thanks for describing the test results.
>
> Committed:
>    1/4:  ee6b7f5bb...
>    2/4:  5047dd4a3...
>    3/4:  ad8ac4b89...
>    4/4:  0dabe1eeb...
>
>
>