chipsalliance / f4pga-v2x

Tool for converting specialized annotated Verilog models into XML needed for Verilog to Routing flow.
https://f4pga-v2x.readthedocs.io/en/latest/
Apache License 2.0
10 stars 12 forks source link

Fasm annotation support #32

Open mkurc-ant opened 4 years ago

mkurc-ant commented 4 years ago

This PR adds a preliminary support for adding fasm annotations.

Supported constructs are:

Documentation how to specify those is added to the vlog_to_pbtype.py header and is also present in README files for tests.

Due to lack support of attributes on parameters in Yosys (they are parsed but discarded and do not end up in a JSON file) support for fasm params is a bit tedious. I'm open to suggestions how to do it in a better way.

mithro commented 4 years ago

Hi @mkurc-ant,

I think we should have a quick chat about this PR.

I think we do want to add fasm support but I would like the fasm features to be actually connected to the verilog names rather than having no relationship to them. The reason I want this is that v2x is suppose to describe the hardware. The FASM features describe configuration of the hardware. Hence there should be a pretty direct mapping between the two.

The goal here would be to be able to tag various things with a generic { fasm } value and then it would end up being used in the construction of the FASM value. For example, putting { fasm } on a module or instance would mean that module's name is used as a value. Tagging a parameter with { fasm } and then use the mode values for that FASM output, etc.

Have you been doing this for the QuickLogic support? If so, it might be a good idea for me to also look at some real world examples to make it easier to make sure I'm not missing something.

Tim '@mithro' Ansell

mkurc-ant commented 4 years ago

@mithro I agree that it would be best to have a one-to-one correspondence between verilog model names and fasm features. But I'm not sure whether that would be possible in all cases. Sometimes the architecture for the VPR need to differ from the physical underlying hardware simple due to the way that the VPR works. Often structure for fasm features in a database also does not exactly reflect the hardware structure. That's why I'd opt for giving a user some freedom for fasm annotation.

And yes, I've implemented the fasm annotation with the QuickLogic support in mind as a first real-world use case of the V2X. You can look how the annotation is done here: https://github.com/antmicro/symbiflow-arch-defs/tree/master%2Bquicklogic/quicklogic/primitives. The annotation is present for LOGIC and BIDIR primitives. Fasm feature names are generated from the vendor database, they are stored in the quicklogic_fasm repo: https://github.com/antmicro/quicklogic-fasm/tree/master/quicklogic_fasm/ql732b

One think I'm not happy about is specification of fasm parameters. Since there is likely be no support for attributes on parameters in the upstream Yosys, parameters cannot be easily annotated. Hence the way of specifying fasm_params i rather tedious and I'm not particularly happy with it as it is done in this PR. But I couldn't think of any better solution.

GitHub
antmicro/symbiflow-arch-defs
FOSS architecture definitions of FPGA hardware useful for doing PnR device generation. - antmicro/symbiflow-arch-defs
GitHub
antmicro/quicklogic-fasm
Contribute to antmicro/quicklogic-fasm development by creating an account on GitHub.
hzeller commented 4 years ago

If the task is to fish out attributes from (System)Verilog, maybe using the Python-API from Surelog might help ?

https://github.com/alainmarcel/Surelog#python-api

With a strategic listener for enterAttribute_instance

GitHub
alainmarcel/Surelog
System Verilog 2017 Pre-processor, Parser . Contribute to alainmarcel/Surelog development by creating an account on GitHub.
mkurc-ant commented 4 years ago

@hzeller It's not that simple. In V2X we use Yosys to convert Verilog modules to JSON representation which is then processed by a Python script. We could use another tool to additionally extract attributes bu then it would create additional dependency for VTX. Not sure if we want that.

mithro commented 4 years ago

@mkurc-ant - Can you give @hzeller's idea a go?

mkurc-ant commented 4 years ago

@mithro Ok, I'll try to integrate that.

So I imagine that it could work in this way: A user specifies eg. the FASM_PARAM attribute on a module parameter and provides the fasm parameter name along with it.

Example:

module MOD();

  (* FASM_PARAM = "FEATURE" *)
  parameter [0:0] PARAMETER = 1'b0;

endmodule

And that would be translated into the following metadata entry:

<meta name="fasm_params">
  FEATURE = PARAMETER
</meta>

Does that make sense?

mkurc-ant commented 4 years ago

This is required to me merged in order for the Surelog integration to work: https://github.com/alainmarcel/Surelog/pull/337. Also a new conda Surelog package with that fix has to be available.

mkurc-ant commented 4 years ago

@mithro @kgugala I've integrated Surelog to extract attributes specified on parameters. PTAL. To test it you need to manually build Surelog with a fix (see the comment above).

mkurc-ant commented 4 years ago

(I think that Travis wasn't triggered for the latest push...)

mithro commented 4 years ago

@mkurc-ant - Can you create a longer term issue to look into if using Surelog rather than Yosys is a better solution for v2x? We should also investigate the idea of v2x being a Yosys Python plugin too (which I think we already have a github issue for?

mkurc-ant commented 4 years ago

@mithro I've created the issue https://github.com/SymbiFlow/python-symbiflow-v2x/issues/64

mkurc-ant commented 4 years ago

@mithro Could you take look at this PR again? I think I addressed all the issues.

Right now it is a hybrid that uses Yosys to parse & elaborate verilog descriptions and Surelog just to get attributes from module parameters (for fasm annotation).

kgugala commented 4 years ago

@mithro can you take a look on this again? This is required for QuickLogic support

mithro commented 4 years ago

@kgugala - I'm still waiting for @mkurc-ant to reply on all the unresolved issues?

The biggest blocking one is https://github.com/SymbiFlow/python-symbiflow-v2x/pull/32#pullrequestreview-369291761

This issue I have with letting people override the name is that people will use the renaming heavily rather than fixing either the fasm or the verilog file.

But there is also multiple others.

mkurc-ant commented 4 years ago

@mithro Could you look at this again? I've addressed all the comments.

I think that we should have a wider discussion about how to express fasm to parameter connection. I've fixed that in this PR so now only the (* FASM *) attribute is supported and it binds a parameter with a fasm feature of the same name. But I have serious doubts about that as it limits the flexibility.

With that change in place we either rework naming of all fasm features or we won't be able to use features which names do not correspond to the hierarchy of modeled cells like eg LIOI3.ILOGIC_Y0.ISERDES.INTERFACE_TYPE.MEMORY_DDR3 for XC7 and X8Y17.LOGIC.LOGIC.INV.TA1 for QuickLogic.

mkurc-ant commented 4 years ago

@mithro Can you take some time and look into my answers to your comments in this PR? Apart from that I've also fixed the CI.

mkurc-ant commented 3 years ago

@mithro I've updated documentation for fasm annotation support. Please review.

I also want to point out an issue with enforcing equal names of FASM features and Verilog parameter names. For example in U-Ray database flip-flop initialization features are named INIT.V0 and INIT.V1 (note the dot). In the current implementation we cannot assign them to Verilog parameters as we would need to have a parameter with a dot in its name which Verilog doesn't allow.

I suggest that we re-evaluate the approach when you can either specify the (* FASM *) attribute on a parameter which would imply identical FASM feature name or provide the FASM feature name explicitly like (* FASM="<feature name>" *). This will allow to model cases like U-Ray FF initialization.

@acomodi FYI

mithro commented 3 years ago

Why not just create a module block called INIT and it has two fasm parameters V0 and V1?

mkurc-ant commented 3 years ago

@mithro So the whole hierarchical name of the feature is CLEL_L.AFF.INIT.V0. We could use that approach but then we would need to pack the FF into the "INIT" pb_type as it is the leaf one.

But there are more features for FFs with different prefixes:

CLEL_L.AFF.INIT.V0
CLEL_L.AFF.INIT.V1
CLEL_L.AFF.SRVAL.V0
CLEL_L.AFF.SRVAL.V1

And here we cannot pack the FF primitive into INIT and SRVAL pb_types simultaneously when differently named features are to be emitted.

mithro commented 3 years ago

@mkurc-ant - What is needed to finish this off?