search

YosysHQ / yosys

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

Use $__ICE40_FULL_ADDER throughout synth_ice40 #1205

Closed eddiehung closed 5 years ago

eddiehung commented 5 years ago

ice40 has a special carry structure that taps the input of a LUTs, rather than the output: image (from Lattice datasheet) thus requiring the LUT input ordering to be preserved.

Currently, in non abc9 flows this done by instantiating a directly SB_CARRY and SB_LUT cell for each adder bit. Then during techmapping, these cells are treated as regular module instantiations and are transformed into PI/POs and thus not touched by the techmapper.

After techmapping, ice40_unlut then converts those SB_LUT cells back into Yosys internal cells $lut and opt_lut then attempts to merge $lut cells together while preserving the input order for those attached to an SB_CARRY.

In #1186, a new $__ICE40_FULL_ADDER cell (encapsulating the SB_CARRY and SB_LUT) was instantiated instead for abc9 flows, necessary because abc9 (a) now has knowledge of the logic and delay through these cells; and (b) the has the capability to optimise either of them away independently. Thus, there was a need to wrap them together as one all-or-nothing atomic unit for optimisation.

As suggested by @whitequark here it may be effective to utilise $__ICE40_FULL_ADDER beyond abc9.

I don't believe it will make any difference to abc (or any other techmappers in their current form) but it could help clean up some of the code surrounding ice40_opt, ice40_unlut, opt_lut, etc.

Also necessary is a way to identify SB_CARRY and SB_LUT patterns that are instantiated by the user (rather than inferred from behavioural code) so that they also get the same treatment.

eddiehung commented 5 years ago

1266 reverted by #1280.

eddiehung commented 5 years ago

According to @smunaut in comment here: https://github.com/YosysHQ/yosys/pull/1280#issuecomment-520164734 Setting the CATCH_MISALIGN parameter to zero is enough to cause issues with #1266. I can't fit picosoc onto a hx8k, but I did use nextpnr's picorv32 wrapper (which has that parameter value set) and it worked for me. Can you share some more details on what you're doing and what you're seeing, please?