Skip to content
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

Publish Surelog builds to PyPi? #2635

Closed
mithro opened this issue Feb 19, 2022 · 32 comments · Fixed by chipsalliance/UHDM#895
Closed

Publish Surelog builds to PyPi? #2635

mithro opened this issue Feb 19, 2022 · 32 comments · Fixed by chipsalliance/UHDM#895
Assignees

Comments

@mithro
Copy link
Contributor

mithro commented Feb 19, 2022

As Surelog has a complete Python API, does it make sense for Surelog to be available via PyPi? If so it would be great to automatically publish PyPi packages.

@alaindargelas
Copy link
Collaborator

alaindargelas commented Feb 19, 2022

Although Surelog Python API has some use for writing pre-processing and parser AST-level rules (Following the Antlr grammar),
the most value would be to have a UHDM Python binding.
The last changelist only converts the Tcl generator into a Python generator for the Surelog Python API. What we would need is a UHDM Python generator (written in Python in the UHDM/scripts system).
I'll let @hs-apotell decide if that is a task he can undertake.

@hs-apotell
Copy link
Collaborator

@alaindargelas Need more information on what exactly do you mean by UHDM Python binding? You want to expose all of UHDM thru' parallel classes in Python?

@alaindargelas
Copy link
Collaborator

Correct, that should be done automatically by code generation like the rest of UHDM.

@hs-apotell
Copy link
Collaborator

Let me investigate this. I have never done extensive work on python bindings. I know AWS Boto3 does a pretty decent job. I will study that as an example. Do you know of any other libraries/projects that does anything similar (not necessarily in context to HDL)??

@alaindargelas
Copy link
Collaborator

alaindargelas commented Feb 19, 2022

Surelog does C to Python binding itself for the antlr AST api using the python script you just ported from TCL along with swig. it should be a very similar approach, there is no need for additional dependency in my mind. From the UHDM model, you auto generate what is required for swig to do the binding to Python.

@alaindargelas
Copy link
Collaborator

If you create a file uhdm.i and saveit in UHDM/build/generated

/* File: uhdm.i */
%module uhdm
%include "typemaps.i"
%include "std_string.i"
%include "std_vector.i"
%include "uhdm/package.h"
%include "uhdm/uhdm.h"

cd UHDM/build/generated
swig -c++ -python -I. -o uhdm_wrap.cxx uhdm.i
uhdm/package.h:41: Error: Syntax error in input(1).

If we use syntax in the C++ headers of UHDM that Swig understands, then the Python wrappers come for "free".

@alaindargelas
Copy link
Collaborator

A more complete .i file produces a Python wrapper that makes sense, it contains the classes enumerated.
You have to remove all the C++ syntax SWIG does not understand like all the RTTI stuff to get SWIG to produce the Python wrapper.

/* File: uhdm.i */
%module uhdm
%include "typemaps.i"
%include "std_string.i"
%include "std_vector.i"    
%include "uhdm/BaseClass.h"
%include "uhdm/scope.h"
%include "uhdm/instance.h"
%include "uhdm/package.h"
%include "uhdm/uhdm.h"

@alaindargelas
Copy link
Collaborator

The other part Surelog that sould be wrapped in Python is the design parsing and command line options "The main".

a Python session should be:

   surelog.init()         << specify and compile the design
   surelog.addFile "... "
   surelog.parse()
   design = surelog.compile()  << get UHDM design object
   foreach  module in design
      ... 

@hs-apotell
Copy link
Collaborator

Appreciate the pointers. I will try the approach. It might not be for the next few weeks though. Current focus is on LS and Linter implementation.

@hs-apotell hs-apotell self-assigned this Feb 20, 2022
@alaindargelas
Copy link
Collaborator

No problem, I don't think there is any urgency. For SWIG to be able to read the UHDM headers "as-is", It think some simple

#ifdef SWIG 
.... 
#endif

in a few places like some of the RTTI class members, and the "final" keyword for classes will be enough. So the same exact headers can be used for the C++ compilation and the SWIG python binding generation.

@alaindargelas
Copy link
Collaborator

BTW, are you going to contribute back the Linter or the LS? Have you noted the UhdmLinter and SynthSubset optional classes in UHDM?

@hs-apotell
Copy link
Collaborator

Moving the LS conversation to #1712

@alaindargelas
Copy link
Collaborator

@hzeller FYI

@alaindargelas
Copy link
Collaborator

With the following SWIG file uhdm.i ran in UHDM/include:

%module uhdm
%{
#include "sv_vpi_user.h"
%}
%include <sv_vpi_user.h>

swig -I. -python -o uhdm_wrap.c uhdm.i
gcc -fPIC -I. -I/usr/include/python3.9/ -c uhdm_wrap.c
gcc -shared uhdm_wrap.o -o uhdmpython.so

You can produce the VPI API in Python. Link uhdmpython.so and uhdm.so in Python, plus a little wrapper for loading the surelog.uhdm and you should have the python binding of the "C" VPI UHDM. The C++ is much more work as described above.

@mithro
Copy link
Contributor Author

mithro commented Mar 24, 2022

FYI - @proppy / @hzeller / @kgugala

@Thomasb81
Copy link
Collaborator

#2635 (comment)

I try to follow the provided instruction but it failed :

>>> import uhdmpython
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ImportError: uhdmpython.so: undefined symbol: vpi_register_assertion_cb
>>>

I found suspicious that even a dummy implementation of vpi_register_assertion_cb is not provided by UHDM.
But also that
nm uhdmpython.so | grep vpi_scan
return nothing ...

Is the proposed swig file correct ?

@hs-apotell
Copy link
Collaborator

UHDM & Surelog packages are not published. This was a discussion about approach and was never followed through. Work on this will begin sometime in near future.

@Thomasb81
Copy link
Collaborator

Hello

In my trial to follow:

You can produce the VPI API in Python. Link uhdmpython.so and uhdm.so in Python, plus a little wrapper for loading the surelog.uhdm and you should have the python binding of the "C" VPI UHDM. The C++ is much more work as described above.

UHDM.i:

%module uhdm
%{
#include "sv_vpi_user.h"
#include "swig_main.h"
%}
%include "std_vector.i"
%include "sv_vpi_user.h"
%include "swig_main.h"

%template(vpiHandleVector) std::vector<vpiHandle>;

swig_main.h:

#include <vector>
#include <uhdm/uhdm.h>

std::vector<vpiHandle> read_uhdm(std::string filename);

swig_main.cpp:

#include "swig_main.h"

std::vector<vpiHandle> read_uhdm(std::string filename){
  UHDM::Serializer serializer;
  return serializer.Restore(filename);
}

The swig cpp wrapper does not compile at my end with a weird error: related to this line of code %template(vpiHandleVector) std::vector<vpiHandle>; in my interface file:

/home/tom/prog/git/Surelog/third_party/UHDM/dbuild/python/CMakeFiles/swig_example.dir/UHDMPYTHON_wrap.cxx:3359:76: error: ‘type_name’ is not a member of ‘swig::traits<unsigned int>’
 3359 | urn traits<typename noconst_traits<Type >::noconst_type >::type_name();
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~

This error seems to me an inconsistency in swig generated code...

I am using Ubuntu : Ubuntu 22.10, python 3.10.7, swig 4.0.2.
I see in swig release note that support to python 3.9 to python 3.11 and support of c++ 17 has only been added in swig 4.1.0 release in October 2022.

I should retry after updating swig to a later version.
But I am wondering if swig is still the right way to build this python wrapper ? (Availability of this swig version in workstation distribution, availability in the CI build system ...)

@Thomasb81
Copy link
Collaborator

Problem remain with swig 4.1.0

@hs-apotell
Copy link
Collaborator

Could you post your changes as a draft PR for me to try locally? Optionally, push them to your own fork and give me access.
Instructions on how to reproduce the issue would help!

@Thomasb81
Copy link
Collaborator

Thomasb81 commented Feb 19, 2023

@hs-apotell
I push the material here : https://github.com/Thomasb81/UHDM/tree/python_wrapper

The python/CMakeLists.txt with swig_add_library is not clean and have dependency issue on generated files. So from scratch if generated files does not exist it does not work.
So actually I work on this with

1. make debug
2. uncomment last line of the root CMakeLists.txt : #add_subdirectory(python)
3. cd dbuild
4. cmake .. -DBUILD_SHARED_LIBS:BOOL=ON -DCMAKE_BUILD_TYPE=debug
5. make swig_example

Edit: after Thomasb81/UHDM@1338905 commit
To load the python module:

  1. cp python/uhdm.py bin/
  2. cd bin
  3. to check proper python module load : python3 -m uhdm

@hs-apotell
Copy link
Collaborator

Add these lines before the %template(vpiHandleVector) in UHDM.i

%include stl.i

namespace std {
  %template(UIntVector) vector<unsigned int>;
}

@hs-apotell
Copy link
Collaborator

BTW, there are lot of examples here on how to use swig for Python code generation.

https://github.com/swig/swig under Examples/python directory.

Templated code can be challenging so hopefully, the resource can assist you. You can always post more questions on the forum.

@Thomasb81
Copy link
Collaborator

namespace std {
  %template(UIntVector) vector<unsigned int>;
}

This code does not make sense to me ... but solve the compilation problem.
Thanks

@Thomasb81
Copy link
Collaborator

In the vpi_user.h we have following function prototype:

PLI_INT32 vpi_vprintf (PLI_BYTE8 *format,va_list ap);
PLI_INT32 vpi_mcd_vprintf (PLI_UINT32 mcd, PLI_BYTE8 *format, va_list ap);

This prototype produce following error at compilation time in there respective wrapper generated code:

UHDMPYTHON_wrap.cxx: In function ‘PyObject* _wrap_vpi_vprintf__SWIG_1(PyObject*, Py_ssize_t, PyObject**)’:
../UHDMPYTHON_wrap.cxx:10237:12: error: invalid array assignment
10237 |       arg2 = *temp;
      |       ~~~~~^~~~~~~

with in the generated code:

SWIGINTERN PyObject *_wrap_vpi_vprintf__SWIG_1(PyObject *SWIGUNUSEDPARM(self), Py_ssize_t nobjs, PyObject **swig_obj) {
...

 va_list arg2 ;
...
 void *argp2 ;
...
  res2 = SWIG_ConvertPtr(swig_obj[1], &argp2, SWIGTYPE_p_va_list,  0  | 0);
...
  va_list * temp = reinterpret_cast< va_list * >(argp2);
  arg2 = *temp;
...
  result = (PLI_INT32)vpi_vprintf(arg1,arg2);
...
}

It seems to me that the generated C++ code is wrong as claim by the compiler.
Not very clear how example and documentation should help me.

Support of va_list in swig 4.0

What is the difference of vpi_printf vs vpi_vprintf. It seems to me that both are variadic function.
Do we need to support both of them in a python wrapper ? Apparently support of vpi_printf seems ok.

@alaindargelas
Copy link
Collaborator

You can comment out both functions in vpi_user.h, just leave a comment that they pose an issue with the SWIG generation, neither have an implementation in C at this point. They are not really needed. That are part of the VPI standard put one can print object content any way they choose to.

@Thomasb81
Copy link
Collaborator

I find an elegant way to exclude them with #ifndef SWIG / #endif arround declaration definition

@Thomasb81
Copy link
Collaborator

I am confuse... does SV API suppose to be usable to modify or create uhdm db or is the intent is limited to get api ?

@alaindargelas
Copy link
Collaborator

The VPI API (C API) is ready only (as it is supposed to be).

We can add a couple of new functions (Should not be in vpi_user.h) to

  • create (With Surelog also linked) the UHDM db
    or
  • load (Only link UHDM) the UHDM db

@Thomasb81
Copy link
Collaborator

Thomasb81 commented Mar 29, 2023

load (Only link UHDM) the UHDM db

Actually the solution provide by chipsalliance/UHDM#895, is to map of uhdm model class because the vpi api only does not allow to update the uhdm db for usecase where uhdm db need to be change (elaboration step, or design mutation)

create (With Surelog also linked) the UHDM db

This is another enhancement that could probably be considered in the future. But it need probably to be done in Surelog not in UHDM.

@alaindargelas
Copy link
Collaborator

I missed some comments here.
I think most users of this would rely on a built model and use the model in Python. Elaboration (-elabuhdm command in Surelog) is done in C++, you don't want to perform UHDM elaboration through python... Or if you want you can expose a simple function that invokes the optional uhdmelab.

Yes, there could be some use cases for mutation, but that also could be accommodated by creating a few new "C" routines that edit the model.

At any rate, we will know soon enough if chipsalliance/UHDM#895 succeed.

If it does, there are still users who would rather use the Standard documented VPI "C" API rather than the C++ one. Basically exposing sv_vpi_user.h to Python.

@timkpaine
Copy link
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants