Skip to content

tree-sitter-systemverilog fix attempt#7279

Merged
kbieganski merged 2 commits intochipsalliance:masterfrom
antmicro:tree-sitter-fix
Jul 17, 2025
Merged

tree-sitter-systemverilog fix attempt#7279
kbieganski merged 2 commits intochipsalliance:masterfrom
antmicro:tree-sitter-fix

Conversation

@tgorochowik
Copy link
Member

@tgorochowik tgorochowik commented Jul 1, 2025

Ref #7264

This PR should fix the installation and invocation of tree sitter systemverilog

@tgorochowik
Copy link
Member Author

With the changes in this PR, tree-sitter-systemverilog seems to be working as expected.

However tree-sitter-verilog tries to execute tree-sitter-verilog as well (and fails) - @gmlarumbe shouldn't tree-sitter-verilog be converted to something similar as your systemverilog work? (i.e. tree-sitter parse, but with a different language selected - is that doable?)

@gmlarumbe
Copy link
Contributor

Hi @tgorochowik ,

Thanks for taking the time to fix the integration of tree-sitter-systemverilog.

Regarding tree-sitter-verilog you are right. It was relying previously on Python bindings for tree-sitter but using the tree-sitter-cli makes more sense to me. Probably replacing runner tree_sitter_verilog.py with something almost similar to tree_sitter_systemverilog.py runner would fix it.

Feel free to change it to see if it works.

On the other hand, the $HOME/.config/tree-sitter/config.json adds $HOME/dev to the list of parser directories and probably both parsers will be suitable to parse the same code. To select one or the other you can try running the following command for each of the runners:

tree-sitter parse --scope "source.verilog" <filename>
tree-sitter parse --scope "source.systemverilog" <filename>

Hope this helps.

@gmlarumbe
Copy link
Contributor

import os
import shutil

from BaseRunner import BaseRunner


class tree_sitter_verilog(BaseRunner):
    def __init__(self):
        super().__init__(
            "tree-sitter-verilog", "tree-sitter", {"parsing"})

        self.submodule = "third_party/tools/tree-sitter-verilog"
        self.url = f"https://github.com/tree-sitter/tree-sitter-verilog/tree/{self.get_commit()}"

    def prepare_run_cb(self, tmp_dir, params):
        self.cmd = [
            self.executable, 'parse', '--scope', 'source.verilog'
        ]
        self.cmd += params['files']

Replacing current tree_sitter_verilog.py runner contents with this snippet should fix it.

@drom
Copy link
Member

drom commented Jul 10, 2025

@tgorochowik @gmlarumbe
Why are you hijacking tree-sitter-verilog project and replacing it with unrelated tree-sitter-systemverilog package?

@gmlarumbe
Copy link
Contributor

Hi @drom,

AFAIK tree-sitter-verilog is not being replaced.

The issues/PRs that modify tree-sitter-verilog runner have to do with the integration of tree-sitter-systemverilog grammar which requires a newer version of tree-sitter. Updating tools and the environment to support tree-sitter-systemverilog resulted in unwanted errors in tree-sitter-verilog and a common approach that shared tools between both grammars was supposed to be the best option (i.e. tree-sitter-cli). You can check #6077 if you are curious.

On the other hand I would be more than happy if you decided to take over the update of your grammar here. I was doing it so far since it seemed unmaintained, with many issues and PRs opened and with no answer for more than 2 years.

@drom
Copy link
Member

drom commented Jul 10, 2025

I don't see tree-sitter-verilog in the table anymore
image

@gmlarumbe
Copy link
Contributor

This probably has something to do with it:

However tree-sitter-verilog tries to execute tree-sitter-verilog as well (and fails) - [...]

But probably @tgorochowik can tell you better.

@kbieganski
Copy link
Collaborator

@drom It was a bug. It's been fixed.

Jakub Wasilewski added 2 commits July 17, 2025 15:28
`tree-sitter parse` expects the grammar.json in `$CWD/src/grammar.json`.
The previous `runners.mk` copied the generated `src/` directory, with the
grammar, to `out/tmp`, which resulted in failing tests, as runners
execute in a random out/tmp/tmp_dir.

* Move generated `src/` directory under
  `out/tmp/tree-sitter-systemverilog/parser` and set shell env variable
  indicating this directory.

* Symlink the parser directory to tmp_dir/src in
  `tree_sitter_systemverilog.py` runner, so `tree-sitter parse` can find
  the grammar.

Signed-off-by: Jakub Wasilewski <jwasilewski@internships.antmicro.com>
Simplify `tree_sitter_verilog.py runner, to leverage the `tree-sitter`
executable instead of `.so` library, for parsing.

Signed-off-by: Jakub Wasilewski <jwasilewski@internships.antmicro.com>
@github-actions
Copy link
Contributor

Changes In Tests

Tool New Failures New Passes Added Removed Not Affected
Odin 0 0 0 0 5051
Yosys 0 1 0 0 4772
tree_sitter_verilog 0 0 0 0 4960
Verilator 0 0 0 0 5129
Icarus 0 0 0 0 5129
sv_parser 0 0 0 0 5051
Slang 0 0 0 0 5114
SynligYosys 0 0 0 0 4772
VeribleExtractor 0 0 0 0 4960
moore_parse 0 0 0 0 4960
Sv2v_zachjs 0 0 0 0 5114
circt_verilog 0 0 0 0 5111
Surelog 0 0 0 0 5114
yosys_slang 0 0 0 0 4306
Verible 0 0 0 0 4960
Slang_parse 0 0 0 0 5051
tree_sitter_systemverilog 301 3930 0 0 726
moore 0 0 0 0 5051

Download an archive containing all the details

@kbieganski kbieganski merged commit b8d6818 into chipsalliance:master Jul 17, 2025
19 checks passed
@kbieganski kbieganski deleted the tree-sitter-fix branch July 17, 2025 15:42
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

Successfully merging this pull request may close these issues.

4 participants