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

Sail Crosscheck Testing with ACT Against Spike #481

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions .github/workflows/apt-packages.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
opam
zlib1g-dev
pkg-config
libgmp-dev
z3
device-tree-compiler
build-essential
python3-setuptools
84 changes: 84 additions & 0 deletions .github/workflows/arch-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,84 @@
name: arch-test

on:
push:
branches:
- master
pull_request:
branches:
- master

jobs:
model-testing:
runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2
with:
fetch-depth: 0

- name: Install Dependencies
run: |
sudo xargs apt-get install -y < .github/workflows/apt-packages.txt
pip3 install git+https://github.com/riscv/riscof.git
wget -c https://github.com/riscv-collab/riscv-gnu-toolchain/releases/download/2024.04.12/riscv64-elf-ubuntu-22.04-gcc-nightly-2024.04.12-nightly.tar.gz
tar -xzf riscv64-elf-ubuntu-22.04-gcc-nightly-2024.04.12-nightly.tar.gz
echo $GITHUB_WORKSPACE/riscv/bin >> $GITHUB_PATH

- name: Build spike
run: |
ci-tests/build-spike
echo $GITHUB_WORKSPACE/install/bin >> $GITHUB_PATH

- name: Build Sail
run: |
ci-tests/build-sail
echo $GITHUB_WORKSPACE/c_emulator >> $GITHUB_PATH

- name: Init arch-tests
run: |
cd ci-tests/riscof
git clone https://github.com/riscv-non-isa/riscv-arch-test
cd riscv-arch-test && git fetch --tags && git checkout tags/3.8.10

- name: Run RV32E
run: |
cd ci-tests/riscof
sed -i 's/\(ispec=\)\(.*\)/\1spike\/spike_isa32e.yaml/' config.ini
./run-tests.sh rv32e_work

- name: Upload Artifacts
uses: actions/upload-artifact@v3
with:
name: artifacts_rv32e
path: |
ci-tests/riscof/rv32e_work/report.html
ci-tests/riscof/rv32e_work/style.css

- name: Run RV32I
run: |
cd ci-tests/riscof
sed -i 's/\(ispec=\)\(.*\)/\1spike\/spike_isa32.yaml/' config.ini
./run-tests.sh rv32i_work

- name: Upload Artifacts
uses: actions/upload-artifact@v3
with:
name: artifacts_rv32i
path: |
ci-tests/riscof/rv32i_work/report.html
ci-tests/riscof/rv32i_work/style.css

- name: Run RV64I
run: |
cd ci-tests/riscof
sed -i 's/\(ispec=\)\(.*\)/\1spike\/spike_isa64.yaml/' config.ini
./run-tests.sh rv64i_work

- name: Upload Artifacts
uses: actions/upload-artifact@v3
with:
name: artifacts_rv64i
path: |
ci-tests/riscof/rv64i_work/report.html
ci-tests/riscof/rv64i_work/style.css
8 changes: 4 additions & 4 deletions build_simulators.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ function test_build () {
fi
}

test_build make ARCH=RV32 ocaml_emulator/riscv_ocaml_sim_RV32
test_build make ARCH=RV64 ocaml_emulator/riscv_ocaml_sim_RV64
test_build make -j"$(nproc 2> /dev/null || sysctl -n hw.ncpu)" ARCH=RV32 ocaml_emulator/riscv_ocaml_sim_RV32
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This needs a comment at least. Why not just nproc?

test_build make -j"$(nproc 2> /dev/null || sysctl -n hw.ncpu)" ARCH=RV64 ocaml_emulator/riscv_ocaml_sim_RV64

test_build make ARCH=RV32 c_emulator/riscv_sim_RV32
test_build make ARCH=RV64 c_emulator/riscv_sim_RV64
test_build make -j"$(nproc 2> /dev/null || sysctl -n hw.ncpu)" ARCH=RV32 c_emulator/riscv_sim_RV32
test_build make -j"$(nproc 2> /dev/null || sysctl -n hw.ncpu)" ARCH=RV64 c_emulator/riscv_sim_RV64
8 changes: 8 additions & 0 deletions ci-tests/build-sail
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# !bin/bash

set -e

opam init --disable-sandboxing -y
eval $(opam config env)
opam install -y sail
./build_simulators.sh
12 changes: 12 additions & 0 deletions ci-tests/build-spike
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
#!/bin/bash
set -e

rm -rf riscv-isa-sim install
mkdir install
git clone https://github.com/riscv-software-src/riscv-isa-sim
cd riscv-isa-sim
mkdir build && cd build
CXXFLAGS="-Wnon-virtual-dtor" CFLAGS="-Werror -Wignored-qualifiers -Wunused-function -Wunused-parameter -Wunused-variable" ../configure --prefix=`pwd`/../../install
make -j"$(nproc 2> /dev/null || sysctl -n hw.ncpu)"
make check
make install
14 changes: 14 additions & 0 deletions ci-tests/riscof/config.ini
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[RISCOF]
ReferencePlugin=sail_cSim
ReferencePluginPath=sail_cSim
DUTPlugin=spike
DUTPluginPath=spike

[spike]
pluginpath=spike
ispec=spike/spike_isa32.yaml
pspec=spike/spike_platform.yaml
target_run=1

[sail_cSim]
pluginpath=sail_cSim
23 changes: 23 additions & 0 deletions ci-tests/riscof/run-tests.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
# !bin/bash
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space in between '#' and '!' here


set -e

# Check if argument (work directory name) provided
if [ $# -eq 0 ]; then
echo "Usage: $0 <argument>"
exit 1
fi

riscof -v debug run --config=config.ini \
--suite=riscv-arch-test/riscv-test-suite/ \
--env=riscv-arch-test/riscv-test-suite/env \
--no-browser --work-dir "$1"

if grep -rniq "$1"/report.html -e '>0failed<'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sort of thing is very fragile in my experience. It's going to say it succeeded if it can't find "failed" which could be for any number of reasons.

Surely riscof has a more robust output method?

then
echo "Test successful!"
exit 0
else
echo "Test FAILED!"
exit 1
fi
17 changes: 17 additions & 0 deletions ci-tests/riscof/sail_cSim/env/link.ld
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
OUTPUT_ARCH( "riscv" )
ENTRY(rvtest_entry_point)

SECTIONS
{
. = 0x80000000;
.text.init : { *(.text.init) }
. = ALIGN(0x1000);
.tohost : { *(.tohost) }
. = ALIGN(0x1000);
.text : { *(.text) }
. = ALIGN(0x1000);
.data : { *(.data) }
.data.string : { *(.data.string)}
.bss : { *(.bss) }
_end = .;
}
33 changes: 33 additions & 0 deletions ci-tests/riscof/sail_cSim/env/model_test.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
#ifndef _COMPLIANCE_MODEL_H
#define _COMPLIANCE_MODEL_H

#define RVMODEL_HALT \
li x1, 1; \
write_tohost: \
sw x1, tohost, x3; \
j write_tohost;

#define RVMODEL_BOOT

#define RVMODEL_DATA_BEGIN \
RVMODEL_DATA_SECTION.align 4; \
.global begin_signature; \
begin_signature:

#define RVMODEL_DATA_END \
.align 4; \
.global end_signature; \
end_signature:

#define RVMODEL_IO_INIT
#define RVMODEL_IO_WRITE_STR(_R, _STR)
#define RVMODEL_IO_CHECK()
#define RVMODEL_IO_ASSERT_GPR_EQ(_S, _R, _I)
#define RVMODEL_IO_ASSERT_SFPR_EQ(_F, _R, _I)
#define RVMODEL_IO_ASSERT_DFPR_EQ(_D, _R, _I)
#define RVMODEL_SET_MSW_INT
#define RVMODEL_CLEAR_MSW_INT
#define RVMODEL_CLEAR_MTIMER_INT
#define RVMODEL_CLEAR_MEXT_INT

#endif
146 changes: 146 additions & 0 deletions ci-tests/riscof/sail_cSim/riscof_sail_cSim.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
import os
import re
import shutil
import subprocess
import shlex
import logging
import random
import string
from string import Template

import riscof.utils as utils
from riscof.pluginTemplate import pluginTemplate
import riscof.constants as constants
from riscv_isac.isac import isac

logger = logging.getLogger()

class sail_cSim(pluginTemplate):
__model__ = "sail_c_simulator"
__version__ = "0.5.0"

def __init__(self, *args, **kwargs):
sclass = super().__init__(*args, **kwargs)

config = kwargs.get('config')
if config is None:
logger.error("Config node for sail_cSim missing.")
raise SystemExit(1)
self.num_jobs = str(config['jobs'] if 'jobs' in config else 1)
self.pluginpath = os.path.abspath(config['pluginpath'])
self.sail_exe = { '32' : os.path.join(config['PATH'] if 'PATH' in config else "","riscv_sim_RV32"),
'64' : os.path.join(config['PATH'] if 'PATH' in config else "","riscv_sim_RV64")}
self.isa_spec = os.path.abspath(config['ispec']) if 'ispec' in config else ''
self.platform_spec = os.path.abspath(config['pspec']) if 'ispec' in config else ''
self.make = config['make'] if 'make' in config else 'make'
logger.debug("SAIL CSim plugin initialised using the following configuration.")
for entry in config:
logger.debug(entry+' : '+config[entry])
return sclass

def initialise(self, suite, work_dir, archtest_env):
self.suite = suite
self.work_dir = work_dir
self.objdump_cmd = 'riscv64-unknown-elf-objdump -D {0} > {1};'
# self.archtest_env = archtest_env.replace("riscv-arch-test", "../my-repo/riscv-arch-test")
self.compile_cmd = 'riscv64-unknown-elf-gcc -march={0} \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I said it years ago for the previous attempt to add something like this to the Sail model and I'll say it again:

  1. Do not hard-code GCC/binutils. Clang/LLVM should be just as well supported, and is arguably a better experience for the end user because they don't need to install a special cross-compiler, as every Clang build can compile for every architecture by default.
  2. The differences between compiling for platforms X and Y are usually minimal, typically some linker script tweaks and providing model_test.h. That should be reflected in the software implementation by having common library code provided by RISCOF to deal with compilers etc, rather than duplicating all that for every single project. Each project should just provide the project-specific compilation information, as well as how to run the test on it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

100% agreed. These riscof generated files are awfully non-generic and the whole design of the framework makes no sense to me. Unfortunately there is no alternative so I guess we will have to include these files in the repository until there is a better solution. Maybe in some hidden .github directory?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been almost 3 years since I reviewed #97 and made the same comments. I don't see why we should accept poor code here just because people responsible for the RISC-V testing frameworks don't action feedback. All this tells me is that the only way to get it done is to push back hard and force them to, otherwise I have no faith that it will ever get fixed and instead just be declared as done and working despite the poor design.

To quote a popular adage, "poor planning on your part does not necessitate an emergency on mine".

-static -mcmodel=medany -fvisibility=hidden -nostdlib -nostartfiles\
-T '+self.pluginpath+'/env/link.ld\
-I '+self.pluginpath+'/env/\
-I ' + archtest_env

# workaround to avoid clang format error in sail-riscv CI.
# *clang suggest style that is syntactically incorrect*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know what it was complaining about, but you can suppress clang-format changes when needed. Please elaborate. But this is not the right approach.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've run into this bug too. clang-format will sometimes reformat assembly into invalid code.

In this case it changes the first two lines to

#define RVMODEL_DATA_SECTION                                                   \
  .pushsection.tohost, "aw", @progbits;                                        \

which I guess is wrong. Anyway I agree, just use // clang-format off and // clang-format on.

# Insert the following macro at run-time.
modelTest_path = 'sail_cSim/env/model_test.h'
macro = """
#define RVMODEL_DATA_SECTION .pushsection \\
.tohost, "aw", @progbits; \\
.align 8; \\
.global tohost; \\
tohost: \\
.dword 0; \\
.align 8; \\
.global fromhost; \\
fromhost: \\
.dword 0; \\
.popsection; \\
.align 8; \\
.global begin_regstate; \\
begin_regstate: \\
.word 128; \\
.align 8; \\
.global end_regstate; \\
end_regstate: \\
.word 4;
"""
with open(modelTest_path, 'r') as file:
modelTest = file.readlines()
modelTest.insert(2, macro)
with open(modelTest_path, 'w') as file:
file.writelines(modelTest)

def build(self, isa_yaml, platform_yaml):
ispec = utils.load_yaml(isa_yaml)['hart0']
self.xlen = ('64' if 64 in ispec['supported_xlen'] else '32')
self.isa = ' ' # 'rv' + self.xlen
ilp32 = 'ilp32e ' if "E" in ispec["ISA"] else 'ilp32 '
self.compile_cmd = self.compile_cmd+' -mabi='+('lp64 ' if 64 in ispec['supported_xlen'] else ilp32)
# if "I" in ispec["ISA"]:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of commented out code around here.

# self.isa += 'i'
# if "M" in ispec["ISA"]:
# self.isa += 'm'
# if "C" in ispec["ISA"]:
# self.isa += 'c'
# if "F" in ispec["ISA"]:
# self.isa += 'f'
# if "D" in ispec["ISA"]:
# self.isa += 'd'

# flag for sail-riscv to enable zcb extension.
# sail model does not yet offer flags for other extensions, supported extensions are enabled by default
if "Zcb" in ispec["ISA"]:
self.isa += ' --enable-zcb'

objdump = "riscv64-unknown-elf-objdump"
if shutil.which(objdump) is None:
logger.error(objdump+": executable not found. Please check environment setup.")
raise SystemExit(1)
compiler = "riscv64-unknown-elf-gcc"
if shutil.which(compiler) is None:
logger.error(compiler+": executable not found. Please check environment setup.")
raise SystemExit(1)
if shutil.which(self.sail_exe[self.xlen]) is None:
logger.error(self.sail_exe[self.xlen]+ ": executable not found. Please check environment setup.")
raise SystemExit(1)
if shutil.which(self.make) is None:
logger.error(self.make+": executable not found. Please check environment setup.")
raise SystemExit(1)

def runTests(self, testList, cgf_file=None):
if os.path.exists(self.work_dir+ "/Makefile." + self.name[:-1]):
os.remove(self.work_dir+ "/Makefile." + self.name[:-1])
make = utils.makeUtil(makefilePath=os.path.join(self.work_dir, "Makefile." + self.name[:-1]))
make.makeCommand = self.make + ' -j' + self.num_jobs
for file in testList:
testentry = testList[file]
test = testentry['test_path']
test_dir = testentry['work_dir']
test_name = test.rsplit('/',1)[1][:-2]

elf = 'ref.elf'

execute = "@cd "+testentry['work_dir']+";"

cmd = self.compile_cmd.format(testentry['isa'].lower(), self.xlen) + ' ' + test + ' -o ' + elf
compile_cmd = cmd + ' -D' + " -D".join(testentry['macros'])
execute+=compile_cmd+";"

# execute += self.objdump_cmd.format(elf, 'ref.disass')
sig_file = os.path.join(test_dir, self.name[:-1] + ".signature")

# execute += self.sail_exe[self.xlen] + ' --test-signature={0} {1} > {2}.log 2>&1;'.format(sig_file, elf, test_name)
execute += self.sail_exe[self.xlen] + ' {0} --no-trace --test-signature={1} {2} > /dev/null;'.format(self.isa, sig_file, elf)

make.add_target(execute)
make.execute_all(self.work_dir)
17 changes: 17 additions & 0 deletions ci-tests/riscof/spike/env/link.ld
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
OUTPUT_ARCH( "riscv" )
ENTRY(rvtest_entry_point)

SECTIONS
{
. = 0x80000000;
.text.init : { *(.text.init) }
. = ALIGN(0x1000);
.tohost : { *(.tohost) }
. = ALIGN(0x1000);
.text : { *(.text) }
. = ALIGN(0x1000);
.data : { *(.data) }
.data.string : { *(.data.string)}
.bss : { *(.bss) }
_end = .;
}
Loading