Fix https://github.com/riscv-collab/riscv-gnu-toolchain/issues/1282

[TommyMurphyTM1234]

See here:

• pk32 build fails due to incorrect arch #1282

[TommyMurphyTM1234]

Hi @kito-cheng - apologies but in messing with my forked repo I made some changes that I did not realise would impact this PR. However the additional change is needed as it happens - in order to bump the Spike submodule commit used to address this:

• Spike submodule needs to be bumped? #1285

So, if it's OK, rather than messing further with my forked repo I might leave this change here too and if/when this PR is merged I will close the two related issues:

• pk32 build fails due to incorrect arch #1282
• Spike submodule needs to be bumped? #1285

If there's any issue with this then please let me know.

[patrick-rivos]

FYI the CI currently fails with an out-of-memory error :(
Rebasing on #1287 once it is merged will allow the testsuite to actually run.

[patrick-rivos]

Actually I'm not sure that pk is even being tested by the CI, so passing the testsuite might not be needed as long as you've tested locally.

[TommyMurphyTM1234]

Actually I'm not sure that pk is even being tested by the CI, so passing the testsuite might not be needed as long as you've tested locally.

As mentioned here:

• pk32 build fails due to incorrect arch #1282 (comment)

These builds work:

git clone https://github.com/riscv-collab/riscv-gnu-toolchain issue-1282
cd issue-1282

# bare metal (newlib)
./configure --prefix=`pwd`/installed-tools
make newlib && SIM=spike make build-sim

# bare metal (newlib) multilib
make distclean && rm -rf installed-tools
./configure --prefix=`pwd`/installed-tools --enable-multilib
make newlib && SIM=spike make build-sim

# linux (glibc)
make distclean && rm -rf installed-tools
./configure --prefix=`pwd`/installed-tools
make linux && SIM=spike make build-sim

# linux (glibc) multilib
make distclean && rm -rf installed-tools
./configure --prefix=`pwd`/installed-tools --enable-multilib
make linux && SIM=spike make build-sim

But I just discovered that this one does not because of a mismatch of hard and soft float modules when trying to link pk.

# bare metal (newlib) rv32ima/ilp32
./configure --prefix=`pwd`/installed-tools --with-arch=rv32ima --with-abi=ilp32
make newlib && SIM=spike make build-sim

However:

1. I suspect (and will check) that this was always an issue when building pk and has not been affected by this PR.
2. I don't know why this issue doesn't impact ANY toolchain that does not bundle the libs for the arch/abi's used to compile pk - viz. rv64gc/lp64d and rv32gc/ilp32d - and how the non-multilib toolchains managed to link successfully.

I'll do some more testing.

[TommyMurphyTM1234]

Ok - seems that this is the key here:

• https://github.com/riscv-collab/riscv-gnu-toolchain/blob/6da3855437e8ab7a8272400287186d5242610172/Makefile.in#L194C1-L205C5

ifeq ($(SIM),spike)
# Using spike simulator.
SIM_PATH:=$(srcdir)/scripts/wrapper/spike
SIM_PREPARE:=PATH="$(SIM_PATH):$(INSTALL_DIR)/bin:$(PATH)" PK_PATH="$(INSTALL_DIR)/$(NEWLIB_TUPLE)/bin/" ARCH_STR="$(WITH_ARCH)"
SIM_STAMP:= stamps/build-spike
ifneq (,$(findstring rv32,$(NEWLIB_MULTILIB_NAMES)))
SIM_STAMP+= stamps/build-pk32
endif
ifneq (,$(findstring rv64,$(NEWLIB_MULTILIB_NAMES)))
SIM_STAMP+= stamps/build-pk64
endif
else

So

1. pk32 gets built if the multilib list contains any rv32 lib(s).
2. pk64 gets built if the multilib list contains any rv64 lib(s).
3. I can't tell for sure but I presume that the multilib list will always contain the toolchain's default arch/abi - i.e. either the default of rv64gc/lp64d or the arch/abi specified by configure ... --with-arch=... --with-abi=....
4. pk32 will only successfully link if the toolchain has a multilib for rv32gc/ilp32f.
5. pk64 will only successfully link if the toolchain has a multilib for rv64gc/lp64d.
6. A newlib or Linux toolchain configured with --enable-multilib will meet these criteria as things stand - i.e. t-elf-multilib and t-linux-multilib specify the necessary multilibs.
7. A toolchain configured with a non-default arch/abi (i.e. other than rv64gc/lp64d) using --with-arch=... --with-abi=..., without --enable-multilib or with a non-default multilib list may fail to link pk32 and/or pk64.

The simplest advice seems to be - if you intend to build/use Spike (and pk) then use --enable-multilib?

[TommyMurphyTM1234]

Actually I'm not sure that pk is even being tested by the CI

I'm not sure that Spike/pk is being tested by the CI but it does look like they are built in certain circumstances in order to run the test suite...

• riscv-gnu-toolchain/.github/workflows/build.yaml

  Line 77 in 6da3855

  test-sim:

  test-sim:
    runs-on: ${{ matrix.os }}
    strategy:
      matrix:
        os:     [ubuntu-20.04]
        mode:   [newlib, linux]
        target: [rv64gc-lp64d]
        sim:    [spike]
        exclude:
          - sim: spike
            mode: linux
    steps:
      - uses: actions/checkout@v2

      - name: initialize submodules
        run: |
          git submodule init
          git submodule update --recursive --progress --recommend-shallow

      - name: install dependencies
        run: sudo ./.github/setup-apt.sh

      - name: build toolchain
        run: |
          TARGET_TUPLE=($(echo ${{ matrix.target }} | tr "-" "\n"))
          ./configure --prefix=/opt/riscv --with-arch=${TARGET_TUPLE[0]} --with-abi=${TARGET_TUPLE[1]} --with-sim=${{ matrix.sim }}
          make -j $(nproc) ${{ matrix.mode }}

      - name: make report
        run: make report-${{ matrix.mode }} -j $(nproc)

so passing the testsuite might not be needed as long as you've tested locally.

Yes - I am testing locally with the results as per my previous post.

[TommyMurphyTM1234]

The simplest advice seems to be - if you intend to build/use Spike (and pk) then use --enable-multilib?

Actually - even simpler:

1. If you want to build pk64 then the toolchain must bundle libraries for rv64gc/lp64d.
2. If you want to build pk32 then the toolchain must bundle libraries for rv32gc/ilp32f.

Either natively or via a multilib,

The substantive change in this PR - i.e. using rv32gc/ilp32f instead of rv32imafdc/ilp32f - is still needed because the former subsumes/infers zifencei while the latter does not and without zifencei compilation fails.

[TommyMurphyTM1234]

Can this be merged?

[patrick-rivos]

LGTM from a CI point of view - the failures should go away once merged since the out of space error was fixed.
I don't have permission to merge - @kito-cheng can you merge it?