Remove dependency on srecord

[Timmmm]

The linked issue was resolved 6 years ago so it's probably safe to use the new flag.

Draft because I'm not sure how to actually test this. Is it used anywhere with Verilator?

[elliotb-lowrisc]

Hi Tim, thanks for this work.

The main things I'd like to check are whether these changes will also work with the lowRISC toolchain and matching the existing style in "examples/sw/simple_system/common/common.mk" a bit better. I believe the lowRISC toolchain objcopy is riscv32-unknown-elf-objcopy in "examples/sw/benchmarks/coremark/ibex/core_portme.mak" and $(OBJCOPY) in "examples/sw/simple_system/common/common.mk". I'll try to take a look at the regressions tomorrow to figure out if they check the vmem output.

[elliotb-lowrisc]

I've convinced myself that the "Run quality checks (Lint and DV)" CI job should test these changes.

Could you please make a few style-related tweaks (see inline comments), mark the PR as non-draft, and then I'll try running it through the full CI.

[elliotb-lowrisc]

A couple of tweaks here should make this change fit in better with the existing code. Could you please swap out objcopy for riscv32-unknown-elf-objcopy, use the short version of flags where available (-O and -I) to match the lines above, and wrap the line before column 100?

[Timmmm]

I generally prefer long arguments for scripts, where readability is more important than saving a few keystrokes... I guess -I and -O aren't too bad.

I also changed it so it directly converts from .elf to .vmem - a bit simpler and more regular.

[elliotb-lowrisc]

Again, a few tweaks would be a big help for maintaining style. Could you please replace objcopy with $(OBJCOPY), and use the short version of flags where available (-O and -I) to match the recipe below?

[Timmmm]

Updated

[SamuelRiedel]

This PR is similar to #2062. I am not the expert on these tools, but it might be useful to check out the discussions that were held there as well. Once this PR gets merged, the other one can be closed.

[elliotb-lowrisc]

My apologies @Timmmm, it seems there is more to this dependency than I realised.

From what I can gather following the trail of links from #2062, we will probably have to stick with srec_cat for now after all. It seems that the we'd need Binutils 2.40 or later for these arguments to be supported correctly (lowRISC/opentitan#1107 (comment)), but the RISC-V fork of Binutils used in our toolchain seems to stop at 2.38 (https://github.com/riscvarchive/riscv-binutils-gdb) and we are currently using 2.35 (https://github.com/lowRISC/lowrisc-toolchains/blame/7471191b10e2a7fbb84cf8c8ab84f729a76a5612/sw-versions.sh#L15). This may have been confirmed by the CI failing just now with the error "riscv32-unknown-elf-objcopy: cannot reverse bytes: length of section .debug_info must be evenly divisible by 4", although I'm not entirely sure if it isn't some other issue. Until our toolchain is updated, we probably have to park this change.

That said, I wonder if it would be possible to update the toolchain. @jwnrt would likely be able to comment once he's back from holiday.

[jwnrt]

We are in the (gradual) process of updating our toolchain. I had a PR (lowRISC/lowrisc-toolchains#87) to update binutils which would include the working objcopy, but we found that it required a newer version of Clang to generate the correct marker symbols it needs for disassembly, so that was blocking. We are currently updating Clang to v21 so when that’s done we can update Binutils too.