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

Removed all synopsys translate_off/on pragmas #689

Merged

Conversation

sifferman
Copy link
Contributor

Hello!

According to the IEEE 1364.1-2005 spec 6.3.2, the use of translate_off/translate_on is discouraged as opposed to `ifndef SYNTHESIS. Plus, some newer SystemVerilog frontends such as sv2v, Verible, and PySlint do not have any extra AST support for translate_off, so they may get confused by unsupported unsynthesizable constructs.

Therefore, I changed 200 occurrences of translate_off to `ifndef SYNTHESIS.

I would be happy to hear another perspective if you feel that translate_off is in fact the better option.

Thanks!

@dpetrisko
Copy link
Contributor

Seems reasonable to me. @taylor-bsg

I would suggest to future-proof we use:

ifndef BSG_HIDE_FROM_SYNTHESIS

where in bsg_defines.sv

ifdef SYNTHESIS
'define BSG_HIDE_FROM_SYNTHESIS
endif

Which will let us have per-tool logic in the future if we need to

@taylor-bsg
Copy link
Contributor

@sifferman thanks for your contribution Ethan. Would you be able to modify the pull request as suggested by @dpetrisko? @dpetrisko would you be able to verify that BP and bsg_manycore are cool with this?

@taylor-bsg
Copy link
Contributor

@sifferman

@@ -74,6 +74,7 @@ module bsg_mem_2rw_sync_mask_write_bit_synth #( parameter `BSG_INV_PARAM(width_p
else
b_addr_r <= 'X;

`ifndef BSG_HIDE_FROM_SYNTHESIS
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I saw that this file was missing the ifndef SYNTHESIS, so I added it

@sifferman
Copy link
Contributor Author

Just made the changes!

@taylor-bsg taylor-bsg merged commit 0b8ca3b into bespoke-silicon-group:master Sep 3, 2024
@taylor-bsg
Copy link
Contributor

TY!

@sifferman
Copy link
Contributor Author

Thanks!

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