-
Notifications
You must be signed in to change notification settings - Fork 36
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
Fixing non power of 2 mux generation #242
Fixing non power of 2 mux generation #242
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've run some tests and the general output looks good.
I compared the default Fabric with the current dev branch, also looks good.
I currently try to run an equivalence check of the outputs with yosys, but this seems to run for a long time. But this is more out of curiosity and for future testing.
But there is one thing, it would be nice if you maybe squash your commits together and provide a meaningful commit message, why this was changed.
For changes like this, it could be good to know on a later point, why this was changed.
The repo is default to squash and merge when merging which will use the pull request title as the commit message.
…________________________________
From: Jonas K. ***@***.***>
Sent: 30 October 2024 16:22
To: FPGA-Research-Manchester/FABulous ***@***.***>
Cc: King Chung ***@***.***>; Author ***@***.***>
Subject: Re: [FPGA-Research-Manchester/FABulous] Fixing non power of 2 mux generation (PR #242)
@EverythingElseWasAlreadyTaken requested changes on this pull request.
I've run some tests and the general output looks good.
I compared the default Fabric with the current dev branch, also looks good.
I currently try to run an equivalence check of the outputs with yosys, but this seems to run for a long time. But this is more out of curiosity and for future testing.
But there is one thing, it would be nice if you maybe squash your commits together and provide a meaningful commit message, why this was changed.
For changes like this, it could be good to know on a later point, why this was changed.
—
Reply to this email directly, view it on GitHub [github.com]<https://urldefense.com/v3/__https://github.com/FPGA-Research-Manchester/FABulous/pull/242*pullrequestreview-2405535288__;Iw!!PDiH4ENfjr2_Jw!AL5GKABU8rMkV2Dtn6dwXfyKdatHxtkh50mxBsYH1KgL40NpNhUciynSbI9Exa0x7L8RcPP3oaUHkw2cDFsn7qmb3WgQlvGX$>, or unsubscribe [github.com]<https://urldefense.com/v3/__https://github.com/notifications/unsubscribe-auth/AQF2K6P3MWWSTWKEZTKGMZ3Z6EBT7AVCNFSM6AAAAABQK6AGHGVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDIMBVGUZTKMRYHA__;!!PDiH4ENfjr2_Jw!AL5GKABU8rMkV2Dtn6dwXfyKdatHxtkh50mxBsYH1KgL40NpNhUciynSbI9Exa0x7L8RcPP3oaUHkw2cDFsn7qmb3fwwynrf$>.
You are receiving this because you authored the thread.Message ID: ***@***.***>
|
Sorry, I forgot about this feature! |
07222cf
into
FPGA-Research:FABulous2.0-development
* mux fix * formatting and rename my_mux2 to cus_mux21_buf * rename and update for vhdl * more rename --------- Co-authored-by: King Lok Chung <[email protected]>
Now can correctly generate non power of 2 mux.
I have also updated the name of the my_mux2 to cus_mux21, to make them follow the same naming conversion for the other muxes.