Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Why does build.yaml test-sim test only the rv64gc/lp64d bare-metal/newlib toolchain? #1351

Closed
TommyMurphyTM1234 opened this issue Oct 14, 2023 · 8 comments

Comments

@TommyMurphyTM1234
Copy link
Collaborator

TommyMurphyTM1234 commented Oct 14, 2023

See here:

And an example run here:

Why does the test-sim test only the rv64gc/lp64d bare-metal/newlib toolchain?

Is this considered a sufficient test of the toolchain?

Why is Linux specified here:

but then excluded here:

Is this perhaps some legacy hangover - e.g. from when multiple simulators were used for testing or the Linux toolchain was tested?

Should this seeming redundancy be removed - e.g.:

  test-sim:
    runs-on: ${{ matrix.os }}
    strategy:
      matrix:
        os:     [ubuntu-20.04]
        mode:   [newlib]
        target: [rv64gc-lp64d]
        sim:    [spike]
@TommyMurphyTM1234 TommyMurphyTM1234 changed the title Why does build.yaml test-sim test only the rv64gc/lp64d bare-metal/newlib toolchain? Why does build.yaml test-sim test only the rv64gc/lp64d bare-metal/newlib toolchain? Oct 14, 2023
@cmuellner
Copy link
Collaborator

The test-sim job runs the regression test suite with Spike:

[...]
  test-sim:
[...]
        sim:    [spike]
[...]
          ./configure --prefix=/opt/riscv --with-arch=${TARGET_TUPLE[0]} --with-abi=${TARGET_TUPLE[1]} --with-sim=${{ matrix.sim }}
[...]

The build job runs the regression test suite with QEMU as defined in configure.ac:

AC_ARG_WITH(sim,
        [AS_HELP_STRING([--with-sim=qemu],
                [Sets the base RISC-V Simulator, defaults to qemu])],
        [],
        [with_sim=qemu]
        )

So, we have the following builds that are tested with QEMU:

      matrix:
        os:     [ubuntu-20.04, ubuntu-22.04]
        mode:   [newlib, linux, musl]
        target: [rv32gc-ilp32d, rv64gc-lp64d]
        compiler: [gcc, llvm]
        exclude:
          - mode: musl
            target: rv32gc-ilp32d
          - mode: musl
            compiler: llvm
[...]
      - name: make report
        if: |
          matrix.os == 'ubuntu-20.04'
          && (matrix.mode == 'linux' || matrix.mode == 'newlib')
          && matrix.compiler == 'gcc'
        run: |
          sudo make report-${{ matrix.mode }} -j $(nproc)
[...]

And for Spike we have:

      matrix:
        os:     [ubuntu-20.04]
        mode:   [newlib, linux]
        target: [rv64gc-lp64d]
        sim:    [spike]
        exclude:
          - sim: spike
            mode: linux

I think this is sufficient.

@TommyMurphyTM1234
Copy link
Collaborator Author

Thanks @cmuellner - couldn't/shouldn't the test-sim job be simplified so?

Should this seeming redundancy be removed - e.g.:

  test-sim:
    runs-on: ${{ matrix.os }}
    strategy:
      matrix:
        os:     [ubuntu-20.04]
        mode:   [newlib]
        target: [rv64gc-lp64d]
        sim:    [spike]

@cmuellner
Copy link
Collaborator

Thanks @cmuellner - couldn't/shouldn't the test-sim job be simplified so?

Should this seeming redundancy be removed - e.g.:

  test-sim:
    runs-on: ${{ matrix.os }}
    strategy:
      matrix:
        os:     [ubuntu-20.04]
        mode:   [newlib]
        target: [rv64gc-lp64d]
        sim:    [spike]

Yes, that would be a valid simplification.
I'm not sure if this is an accident, a leftover or intention ("we could run it, but it would take too long so we exclude it").

@TommyMurphyTM1234
Copy link
Collaborator Author

OK - thanks. I'll do a PR for this small change so.

@TommyMurphyTM1234
Copy link
Collaborator Author

Rather than open a separate issue...

Is there any reason why these jobs are run on Ubuntu 20.04 and not 22.04 (both are LTS releases)?

@cmuellner
Copy link
Collaborator

Rather than open a separate issue...

Is there any reason why these jobs are run on Ubuntu 20.04 and not 22.04 (both are LTS releases)?

Sounds like a good PR! ;)

@TommyMurphyTM1234
Copy link
Collaborator Author

Sounds like a good PR! ;)

Ok - I'll do that so. :-) Thanks.

patrick-rivos pushed a commit that referenced this issue Oct 16, 2023
TommyMurphyTM1234 added a commit to TommyMurphyTM1234/riscv-gnu-toolchain that referenced this issue Oct 16, 2023
@TommyMurphyTM1234
Copy link
Collaborator Author

Both issues mentioned above addressed by this PR:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants