[PATCH v3 0/4] microblaze: fix and clean up md_apply_fix and md_assemble

[Sam Price]

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...
>
>
>

-- 
Sincerely,

Sam Price
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://sourceware.org/pipermail/binutils/attachments/20260924/0c1880bf/attachment-0001.htm>