Skip to content

Miscellaneous additions#48

Merged
steveri merged 2 commits into
StanfordVLSI:masterfrom
stfns-s:misc
Apr 29, 2026
Merged

Miscellaneous additions#48
steveri merged 2 commits into
StanfordVLSI:masterfrom
stfns-s:misc

Conversation

@stfns-s
Copy link
Copy Markdown
Contributor

@stfns-s stfns-s commented Apr 27, 2026

  1. ./bin/gvp2.pl: lightweight cpp like pre-processor using Genesis2 libs and semantics. See usage below and ./demo/gvp2/
$ bin/gvp2.pl -h
usage: gvp2.pl [--h] [--libdirs d] [--incdirs d] [--defparam p=v] file(s)
    --h              : This message
    --rawperl|pdebug : Output tidied raw perl for debugging instead of running it
    --mname  name    : Set top module name
    --libdirs d1,d2  : Add dirs to the perl @INC path
    --incdirs d1,d2  : Add dirs to the include search path
    --defparam p=v   : Set parameter 'p' to value 'v'
    --comment str    : Set the comment start of output language to "str"
                       (default "//"; perl-escape becomes "str;")
  1. ./extras/vim/: vim/neovim syntax highlighting. See ./extras/vim/README.md

  2. cleanup: remove dead symlinks in ./gui/designs.aux/savelinks/

@stfns-s
Copy link
Copy Markdown
Contributor Author

stfns-s commented Apr 27, 2026

Small additions. Hope they are useful.

@steveri
Copy link
Copy Markdown
Contributor

steveri commented Apr 27, 2026

Hm one of the checks is failing, can you have a look
https://github.com/StanfordVLSI/Genesis2/actions/runs/24982347187/job/73196101186?pr=48

@stfns-s
Copy link
Copy Markdown
Contributor Author

stfns-s commented Apr 27, 2026

Hi,
I think the pproblem is that the workflow in gold.yml doesn't work for PRs originating from repo forks. The workflow passes $BRANCH_NAME (which is github.head_ref) as the first argument to test/garnet.sh. Then it is captured as $commit on line 32 and used on line 95:

dexec "set -x; cd $REPO; git pull; git checkout -fq $commit" || exit 13

That git checkout runs inside the docker container against a clone of StanfordVLSI/Genesis2. Since this PR originates from my forked repo the misc branch only exists on my fork, so the checkout fails and the job exits before any real testing happens. According to https://docs.github.com/en/actions/learn-github-actions/contexts#github-context, github.head_ref on a pull_request event refers to the branch on the head repo, which isn't reachable from origin when the PR is from a fork — which is why actions/checkout itself defaults to the PR head SHA in that case.

That can be fixed by changing line 10 of .github/workflows/gold.yml:

# From:
env:
  BRANCH_NAME: ${{ github.head_ref || github.ref_name}}
# To:
env:
  BRANCH_NAME: ${{ github.event.pull_request.head.sha || github.sha }}

With that change the PR head SHA is mirrored on origin under refs/pull/N/head, so git pull && git checkout -fq <sha> on line 95 will work for both fork PRs and same-repo pushes.

Could you please apply this change on master so that I can rebase and re-run the regressions on the actual PR ?

Thanks..

@steveri
Copy link
Copy Markdown
Contributor

steveri commented Apr 27, 2026

I will look at this again tomorrow. Meanwhile I would be interested to know...who are you and what is your interest in Genesis2?

Also @grg I would be interested in your take on these proposed changes...

  • Steve

@steveri
Copy link
Copy Markdown
Contributor

steveri commented Apr 28, 2026

Okay @stfns-s I have pushed (to master) a fix for the CI problem. It's a little different than what you proposed, but I think it will work. It's also just a bit hacky, and I may change it in the future, but I think it's what I'm comfortable with for now.

Please pull and rebase and push etc. and we will see what happens.

Also: if you want to include your smoke test as part of gold.yml I would not be opposed...

Thanks!

@steveri steveri self-requested a review April 28, 2026 19:05
@steveri steveri self-assigned this Apr 28, 2026
@steveri
Copy link
Copy Markdown
Contributor

steveri commented Apr 28, 2026

Oops sorry I hit send on previous comment a little too soon, changes were not yet merged to master.
So but now I have done it, so you are clear to try the rebase etc.

Copy link
Copy Markdown
Contributor

@steveri steveri left a comment

Choose a reason for hiding this comment

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

lgtm

@steveri steveri merged commit 1d75669 into StanfordVLSI:master Apr 29, 2026
6 checks passed
Copy link
Copy Markdown
Collaborator

@grg grg left a comment

Choose a reason for hiding this comment

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

I didn't get a chance to review before the merge.

Here's a little feedback anyway 🙂

Comment thread demo/gvp2/example.vp
Comment on lines +3 to +4
//;my $WIDTH = parameter(name => 'WIDTH', val => 8);
//;pinclude("example_lib.pl")
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you make the spacing consistent for the Genesis //; invocations please? You have no space between //; and the perl code here, but on lines 14/16 you have a space.

(I vote for always including the space.)

Comment thread demo/gvp2/Makefile
Comment on lines +5 to +6
GVP2 ?= $(abspath $(HERE)/../../bin/gvp2.pl)
GVP ?= $(abspath $(HERE)/../../gvpy/gvp.pl)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Does this fail if you just reference ../../bin/gvp2.pl (and similarly for gvp.pl)?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Rather than calling it vp, why not call it genesis2? 🙂

Comment thread extras/vim/syntax/vp.vim
@@ -0,0 +1,114 @@
" Vim syntax file
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Any idea how this compares to the two versions referenced here: https://github.com/StanfordVLSI/Genesis2/wiki/Genesis2#user-content-Setting_Genesis2_Mode_for_Vim

I had created one version, but I'm not particularly attached to it. I'm just hoping that this isn't missing anything from the other two if it's being included in the base repo 🙂

(Also probably worth updating the wiki to reflect that there's now a syntax file included in the base repo.)

@steveri
Copy link
Copy Markdown
Contributor

steveri commented Apr 29, 2026

I didn't get a chance to review before the merge.
Here's a little feedback anyway 🙂

Thanks so much for the feedback! I will leave it for @stfns-s to respond.
And. In the future I will try to be more inclusive with the review process.

@stfns-s
Copy link
Copy Markdown
Contributor Author

stfns-s commented Apr 29, 2026

@steveri thanks for merging
@grg Thanks for the feedback!. I will go through the comments, incorporate and perhaps request another merge. Been a while btw, I hope all is well..

@grg
Copy link
Copy Markdown
Collaborator

grg commented Apr 30, 2026

It's cool that you're still using Genesis. Of course I'm still using it too 🙂

@stfns-s stfns-s deleted the misc branch April 30, 2026 18:01
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.

3 participants