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

Fix miss fomratted SV #1237

Merged
merged 1 commit into from
Feb 12, 2025

Conversation

taichi-ishitani
Copy link
Contributor

Emission of omitted ports is copy of port declarations. Therefore, location info of inst declaration and such port connections are different and this causes such miss emitted SV.
To fix it, location info needs to be adjusted.

fix #1229

@taichi-ishitani
Copy link
Contributor Author

Expression portions are not aligned because of the same reason but I'm not sure how to adjust it.

veryl_testcase___Module73A__0 u0 (
.i_a (veryl_testcase_Package73::A),
.i_b (0 ),
.i_c (0),
.o_d ( ),
.o_e ()
);
veryl_testcase___Module73A__1 u1 (
.i_a (veryl_testcase_Package73::A),
.i_b (1 ),
.i_c (0),
.o_d ( ),
.o_e ()
);
veryl_testcase___Module73A__1 u2 (
.i_a (0 ),
.i_b (0 ),
.o_d (_d),
.i_c (0),
.o_e ()
);

Copy link

codspeed-hq bot commented Feb 10, 2025

CodSpeed Performance Report

Merging #1237 will not alter performance

Comparing taichi-ishitani:fix_miss_formated_sv (a3867d1) with master (6568689)

Summary

✅ 3 untouched benchmarks

@dalance
Copy link
Collaborator

dalance commented Feb 11, 2025

I think that changing aligner structure may be necessary.
Aligner ties the amount of align with the token location, but default port value doesn't have the corresponding token.
I'll consider new aligner architecture.

@dalance
Copy link
Collaborator

dalance commented Feb 12, 2025

I'm working on #1245.
In the case of default value only, it was fixed, but combination of explicit and default is not yet.

@dalance dalance force-pushed the fix_miss_formated_sv branch from 513d1a3 to a3867d1 Compare February 12, 2025 03:26
@dalance
Copy link
Collaborator

dalance commented Feb 12, 2025

I'll merge this PR at first, and the remaining issue will be fixed by merging #1245.

@dalance dalance enabled auto-merge February 12, 2025 03:29
@dalance dalance merged commit 614457c into veryl-lang:master Feb 12, 2025
9 checks passed
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.

Miss-formatted SV when omiting port connections
2 participants