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: S-interpolator for assert, assume and printf #3103

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

SingularityKChen
Copy link
Contributor

@SingularityKChen SingularityKChen commented Oct 4, 2022

Related issue: chipsalliance/chisel#2751

Type of change: bug report

Impact: no functional change

Development Phase: implementation

Release Notes

@sequencer
Copy link
Member

@SingularityKChen rebase to master plz

@sequencer
Copy link
Member

LGTM, but I'm not so familiar with printf API, @jackkoenig for review&approve.

@SingularityKChen SingularityKChen changed the base branch from master to dev June 15, 2023 05:35
@sequencer sequencer self-requested a review June 15, 2023 05:51
assert(!q.enq.valid, s"ID ${i} should not be used")
chisel3.assert(!q.enq.valid, "ID ${i} should not be used")
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is correct.

@@ -73,7 +73,8 @@ class TLPatternPusher(name: String, pattern: Seq[Pattern])(implicit p: Parameter
}

val (plegal, pbits) = pattern.map(_.bits(edgeOut)).unzip
assert (end || VecInit(plegal)(step), s"Pattern pusher ${name} tried to push an illegal request")
val msg = s"Pattern pusher $name tried to push an illegal request"
assert(end || Vec(plegal)(step), msg)
Copy link
Member

Choose a reason for hiding this comment

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

VecInit?

Suggested change
assert(end || Vec(plegal)(step), msg)
assert(end || VecInit(plegal)(step), msg)

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.

2 participants