YosysHQ / yosys

Yosys Open SYnthesis Suite
https://yosyshq.net/yosys/
ISC License
3.51k stars 895 forks source link

rtlil: add Const::compress helper function #4608

Closed phsauter closed 1 month ago

phsauter commented 2 months ago

If one has a Sigspec that is fully constant it is currently non-trivial to get its value.
The main blocking point here is that the SigSpec may be wider than 32bit (an integer).
However, this doesn't mean the value stored in it is necessarily this large/small.

Eg something like 33'd4 or 33'd-2 cannot be easily converted to an int.

This adds a compress(is_signed) function.
It tries to minimize the number of bits used for the representation of the number. It does so by removing the leading bits while taking signedness into consideration.

With this function, the above can be successfully converted to an integer by first calling compress.

Edit: Here is a possible application. https://github.com/povik/yosys-slang/blob/28cea56840a6bc8a4b6398d36609500d5960ad79/src/builder.cc#L134
With compress it is easy to check if the actual value has a larger representation than 24-bits instead of checking the SigSpec, which again may not use the minimum width.
I am fairly convinced there are places in the Yosys codebase itself this would be helpful, finding them is a bit difficult.

phsauter commented 2 months ago

Should I also add a .as_int_compress(bool is_signed) to make it even easier to get a constant integer?

povik commented 2 months ago

For that one to be useful you need to know when you can trust the result and when not, so maybe it should return a pair of int, bool. If it can fail, then we may call it try_int.

widlarizer commented 2 months ago

That's what std::optional is for. What could fail?

phsauter commented 2 months ago

If it is larger than 32-bit then it fails as it is not representable but yes, std::optional is a good option there.

phsauter commented 2 months ago

Does this solution look reasonable?

widlarizer commented 2 months ago

As far as I can tell, it does