Skip to content

back.rtlil: Opportunistically trim zero and sign extension on operands. #1152

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

Merged

Conversation

wanda-phi
Copy link
Member

Fixes #1148.

Copy link

codecov bot commented Feb 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.84%. Comparing base (2d59242) to head (f5f3d20).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1152      +/-   ##
==========================================
+ Coverage   90.79%   90.84%   +0.04%     
==========================================
  Files          44       44              
  Lines       10519    10571      +52     
  Branches     2561     2577      +16     
==========================================
+ Hits         9551     9603      +52     
  Misses        777      777              
  Partials      191      191              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@whitequark whitequark left a comment

Choose a reason for hiding this comment

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

This is a little cursed and I'm worried about the potential for bugs, but we have to start doing randomized testing either way...

I would be much happier with at least a few tests, I think.

@wanda-phi
Copy link
Member Author

This is a little cursed and I'm worried about the potential for bugs, but we have to start doing randomized testing either way...

I would be much happier with at least a few tests, I think.

Yeah....

I do want to add some tests for this, but unfortunately this'll have to wait until #1100 lands, we don't really have good infrastructure for that right now.

Copy link
Contributor

@tpwrules tpwrules left a comment

Choose a reason for hiding this comment

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

Works on my machine™! Thank you.

@whitequark
Copy link
Member

Since it's addressing a regresstion I'm not going to block this on #1100, @wanda-phi, your call on when to merge this.

@whitequark
Copy link
Member

Oh, just noticed the branch name :D

@whitequark whitequark added this to the 0.5 milestone Feb 29, 2024
@whitequark whitequark marked this pull request as draft February 29, 2024 19:02
@whitequark whitequark mentioned this pull request Mar 22, 2024
77 tasks
@wanda-phi wanda-phi force-pushed the width-added-and-removed-here branch from a0ab64a to f5f3d20 Compare April 4, 2024 01:50
@wanda-phi wanda-phi marked this pull request as ready for review April 4, 2024 01:50
Copy link
Member

@whitequark whitequark left a comment

Choose a reason for hiding this comment

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

Regretfully, LFGTM.

@whitequark whitequark enabled auto-merge April 4, 2024 01:52
@whitequark whitequark added this pull request to the merge queue Apr 4, 2024
Merged via the queue into amaranth-lang:main with commit d3c5b95 Apr 4, 2024
20 checks passed
@wanda-phi wanda-phi deleted the width-added-and-removed-here branch April 4, 2024 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

New IR causes DSP inference issues with Quartus
3 participants