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

CI workflow for SystemVerilog syntax checking #46

Merged
merged 17 commits into from
Aug 14, 2024
Merged
Show file tree
Hide file tree
Changes from 7 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
95 changes: 95 additions & 0 deletions .github/workflows/check_verilog.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
name: check-verilog
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why check-verilog name is used, if SystemVerilog description is tested?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.


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

env:
CIRCT_VERSION: 1.72.0
CIRCT_ARCHIVE: "circt-full-shared-linux-x64.tar.gz"
DEPENDENCIES_DIR: "deps"
MLIR_TARGETS_PATH: "lib/cmake/mlir/MLIRTargets.cmake"
BUILD_DIR: "build"
UTOPIA_EXECUTABLE: "src/umain"
MODULE_OUTPUT_PATH: "output.sv"
LIBRARY_OUTPUT_PATH: "lib.sv"
Copy link
Collaborator

Choose a reason for hiding this comment

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

These environment variables are used at specific jobs/steps only. It would be better to declare them locally (well, as-local-as-possible).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.


jobs:
build-and-parse-verilog:
Copy link
Collaborator

Choose a reason for hiding this comment

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

First of all, not "verilog" but "systemverilog", or, probably, "sv".

Also this job name looks a bit straightforward. What if one more step, (for example, logic synthesis) will be added here? What name will be used -- "build-and-parse-and-synthesize-systemverilog"? I'd use more general name like "synth-sv" -- our main task is to perform good HLS, and we can omit extra details here and write a describing comment, if needed,

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

strategy:
matrix:
ssmolov marked this conversation as resolved.
Show resolved Hide resolved
kernel: [polynomial2, scalar3, matrixmul2, muxmul, addconst, movingsum]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Probably a lexicographical order would be more deterministic for future kernels adding

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

config: [1]
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does it mean? Numbers aren't illustrative, by the way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed with different configuration naming.

include:
- kernel: polynomial2
config: 2
- kernel: movingsum
config: 2

runs-on: ubuntu-latest
steps:
- name: Check out repository code
uses: actions/checkout@v4

- name: Download APT dependencies
run: |
sudo apt update
sudo apt install build-essential clang cmake g++ gcc liblpsolve55-dev lld make ninja-build libctemplate-dev verilator

- name: Download and configure CIRCT & LLVM
env:
CIRCT_SOURCE: "https://github.com/llvm/circt/releases/download/firtool-${{env.CIRCT_VERSION}}/${{env.CIRCT_ARCHIVE}}"
CIRCT_DIR: "firtool-${{env.CIRCT_VERSION}}"
run: |
mkdir "${{env.DEPENDENCIES_DIR}}"
cd "${{env.DEPENDENCIES_DIR}}"
wget "${{env.CIRCT_SOURCE}}"
tar -xvf "${{env.CIRCT_ARCHIVE}}"
sed -i 's'/\
'foreach(_target "LLVMSupport" "LLVMCore" "LLVMMC" "LLVMTarget" "LLVMAsmParser" "LLVMBinaryFormat" '\
'"LLVMBitReader" "LLVMBitWriter" "LLVMFrontendOpenMP" "LLVMTransformUtils" "LLVMTargetParser" "LLVMIRReader" '\
'"LLVMipo" "LLVMLinker" "LLVMPasses" "LLVMMCParser" "LLVMLineEditor" "LLVMTableGen" "LLVMCoroutines" "LLVMExecutionEngine" '\
'"LLVMObject" "LLVMOrcJIT" "LLVMJITLink" "LLVMAnalysis" "LLVMAggressiveInstCombine" "LLVMInstCombine" "LLVMScalarOpts" '\
'"LLVMVectorize" "LLVMX86CodeGen" "LLVMX86Desc" "LLVMX86Info" "LLVMX86AsmParser" "LLVMX86Disassembler" "CIRCTAffineToLoopSchedule" '\
'"CIRCTArcToLLVM" "CIRCTCalyxToFSM" "CIRCTCalyxToHW" "CIRCTCalyxNative" "CIRCTCombToArith" "CIRCTCombToLLVM" "CIRCTCombToSMT" '\
'"CIRCTConvertToArcs" "CIRCTDCToHW" "CIRCTExportChiselInterface" "CIRCTExportVerilog" "CIRCTFIRRTLToHW" "CIRCTFSMToSV" '\
'"CIRCTHandshakeToDC" "CIRCTHandshakeToHW" "CIRCTHWArithToHW" "CIRCTHWToLLHD" "CIRCTHWToLLVM" "CIRCTHWToBTOR2" "CIRCTHWToSMT" '\
'"CIRCTHWToSV" "CIRCTHWToSystemC" "CIRCTLLHDToLLVM" "CIRCTLoopScheduleToCalyx" "CIRCTMooreToCore" "CIRCTPipelineToHW" "CIRCTSCFToCalyx" '\
'"CIRCTSeqToSV" "CIRCTSimToSV" "CIRCTCFToHandshake" "CIRCTVerifToSMT" "CIRCTVerifToSV" "CIRCTExportFIRRTL" "CIRCTComb" '\
'"CIRCTCombTransforms" "CIRCTDebug" "CIRCTESI" "CIRCTFIRRTL" "CIRCTImportFIRFile" "CIRCTMSFT" "CIRCTMSFTTransforms" "CIRCTHW" '\
'"CIRCTLLHD" "CIRCTMoore" "CIRCTOM" "CIRCTOMEvaluator" "CIRCTSeq" "CIRCTSeqTransforms" "CIRCTSV" "CIRCTSVTransforms" "CIRCTFSM" '\
'"CIRCTFSMTransforms" "CIRCTHandshake" "CIRCTHandshakeTransforms" "CIRCTHWArith" "CIRCTVerif" "CIRCTLTL" "CIRCTEmit" "CIRCTFirtool" )'\
/\
'foreach(_target "LLVMSupport" "LLVMCore" "LLVMMC" "LLVMTarget" "LLVMAsmParser" "LLVMBinaryFormat" "LLVMBitReader" "LLVMBitWriter" '\
'"LLVMFrontendOpenMP" "LLVMTransformUtils" "LLVMTargetParser" "LLVMIRReader" "LLVMipo" "LLVMLinker" "LLVMPasses" "LLVMMCParser" '\
'"LLVMLineEditor" "LLVMTableGen" "LLVMCoroutines" "LLVMExecutionEngine" "LLVMObject" "LLVMOrcJIT" "LLVMJITLink" "LLVMAnalysis" '\
'"LLVMAggressiveInstCombine" "LLVMInstCombine" "LLVMScalarOpts" "LLVMVectorize" "LLVMX86CodeGen" "LLVMX86Desc" "LLVMX86Info" '\
'"LLVMX86AsmParser" "LLVMX86Disassembler")/' "${{env.CIRCT_DIR}}/${{env.MLIR_TARGETS_PATH}}"
cd "${{github.workspace}}"

- name: Configure Utopia HLS build
env:
CIRCT_DIR: "firtool-${{env.CIRCT_VERSION}}"
run: |
cmake -S . -B "${{env.BUILD_DIR}}" -G Ninja -DCMAKE_CXX_COMPILER=clang++ \
-DCMAKE_PREFIX_PATH="${{github.workspace}}/${{env.DEPENDENCIES_DIR}}/${{env.CIRCT_DIR}}" \
-DSRC_FILES="${{github.workspace}}/examples/${{matrix.kernel}}/${{matrix.kernel}}.cpp"

- name: Build Utopia HLS
run: |
cmake --build "${{env.BUILD_DIR}}"

- name: Run SystemVerilog modules generation
run: |
"${{env.BUILD_DIR}}/${{env.UTOPIA_EXECUTABLE}}" hls --config \
"${{github.workspace}}/examples/${{matrix.kernel}}/${{matrix.kernel}}_${{matrix.config}}.json" \
-l --out-sv "${{env.MODULE_OUTPUT_PATH}}" --out-sv-lib "${{env.LIBRARY_OUTPUT_PATH}}"

- name: Run Verilator
run: |
verilator --lint-only -Wall -Wno-DECLFILENAME "${{env.MODULE_OUTPUT_PATH}}" "${{env.LIBRARY_OUTPUT_PATH}}"
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ For example, given subdirectory `polynomial2`, the compilation and execution com
```bash
cmake -S . -B build -G Ninja -DCMAKE_PREFIX_PATH=~/firtool-1.72.0 -DSRC_FILES="~/utopia-hls/examples/polynomial2/polynomial2.cpp"
cmake --build build
build/src/umain hls --config examples/polynomial2/polynomial2.json -a --out-sv output
build/src/umain hls --config examples/polynomial2/polynomial2_1.json -a --out-sv output
Copy link
Collaborator

Choose a reason for hiding this comment

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

What are these "_1" and "_2" filename suffixes for? Looks ugly.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

```

## DFCxx Documentation
Expand Down
File renamed without changes.
File renamed without changes.
File renamed without changes.
3 changes: 3 additions & 0 deletions examples/movingsum/movingsum_2.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"ADD_INT": 8
}
File renamed without changes.
4 changes: 4 additions & 0 deletions examples/polynomial2/polynomial2_2.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"MUL_INT": 15,
"ADD_INT": 8
}
File renamed without changes.