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

quickstart/sharedvar/shrecord example #17

Closed
wants to merge 6 commits into from

Conversation

radonnachie
Copy link

I recently concluded the neatest way to share records/structs and thought to add it to sharedvar

In particular it clarifies that a non access record type is received as a struct* and that struct* is received in vhdl as an access record. Which is a weird asymmetry.

I'd imagine that passing an access-record to a VHPIDIRECT function would lead to receiving a struct** in the C function.

Documented, matching and (hopefully) up to standard!

@umarcor
Copy link
Owner

umarcor commented May 8, 2020

@RocketRoss, thanks! I cannot review this ATM. However...

In particular it clarifies that a non access record type is received as a struct* and that struct* is received in vhdl as an access record. Which is a weird asymmetry.

Yes, I found this while working on the examples about integer vectors and matrices. It seems that we can pass either a constrained array or an access to a contrained array from VHDL to C, and C will receive the same pointer. I believe this is because, in C, arrays are nothing but pointers (except bounded arrays, which support sizeof). For the same reason, C cannot pass anything other than a pointer.

At some point we'll need to clarify this along with:

  • row-major vs column-major ordering
  • endianness

@tgingold, any hint you can provide will be so welcome.

@tgingold
Copy link

tgingold commented May 8, 2020 via email

@radonnachie
Copy link
Author

radonnachie commented May 11, 2020

At some point we'll need to clarify this along with:

• row-major vs column-major ordering
Same as C.
• endianness
No change.

Agreed, I have seen no real differences creep up in all my time.

I am not. What do you mean by 'received in vhdl' ?

impure function c_getStruct return c_struct_ptr is where c_struct_ptr is an access type, and the c function returns a struct*. What I mean to highlight is that the returned struct* does not get passed by reference to produce a struct** or access access.... Clearer?

I need to double check what happens when the function returns a struct
If the c-function returns a struct, none of the fields are correctly accessed in VHDL. So there is an asymmetry: records from VHDL -> C are passed by reference, structs from C -> VHDL are passed by value (so C must pass a struct*, which is handed over by value). I'm adding it to a note in the docs

.. NOTE::
  There is an asymmetry: records from VHDL passed to C are passed by reference, whereas structs from C passed to VHDL are passed
  by value (so C must pass a ``struct*``, which is handed over by value). To further spell out the consequences: VHPIDIRECT subprograms
  take records as their arguments but return record access types.

@tgingold
Copy link

tgingold commented May 11, 2020 via email

@radonnachie
Copy link
Author

IIRC, there is an additional argument to give the address where the result should be written.

That wouldnt change the asymmetry right? I don't mind the asymmetry, just will be good to note in the documentation of the example,.

So there is an asymmetry: records from VHDL -> C are passed by reference, structs from C -> VHDL are passed by value

@umarcor
Copy link
Owner

umarcor commented May 13, 2020

At some point we'll need to clarify this along with:

• row-major vs column-major ordering
Same as C.
• endianness
No change.

Agreed, I have seen no real differences creep up in all my time.

Yes. We need to do nothing but to explicitly write it down somewhere.

@RocketRoss, I think that the asymmetry that you mean and what Tristan is telling about "there is an additional argument to give the address where the result should be written" are two different but related issues:

  • On the one hand, some data types are passed by value and others are passed by reference. When passing arrays/records/structs the value and the reference are both the same pointer, which you dereference or you do not. I think that this is one source of misunderstanding.

For example, in https://github.com/umarcor/ghdl-cosim/blob/master/vhpidirect/arrays/intvector/vhdlsized we are passing an access type to C:

procedure c_initIntArr (variable ptr: int_arr_ptr; arraySize: integer );

We allocate it as:

variable c_intArr : int_arr_ptr := new int_arr;

And we then use it:

c_initIntArr(c_intArr, c_intArr'length);

In some cases, it would be equivalent to do:

procedure c_initIntArr (variable ptr: int_arr; arraySize: integer );
variable c_intArr : int_arr;
c_initIntArr(c_intArr, c_intArr'length);

The C prototype would not change, although from GHDL's perspective one is about passing the ref and the other one is about passing the value.

As you see, and as Tristan told, the strategy that GHDL uses to return a complex type by value, is to add the facade of a pointer to the type. So, if you declare function myproc return my_record_t, the matching prototype in C would be void myproc(my_struct_t* ptr) instead of my_struct_t* myproc(void). However, unlike other "pointers" passed by GHDL, this one is unallocated (because it is the facade of a value, not a pointer). Hence, C is expected to allocate all and fill the fields. By the same token, it should be later deallocated/freed in C.

Honestly, this is where I reached my limit without further help. However, I believe that you are a better C coder than me, and I think that you already solved this in #8. Hence, let's have this PR reviewed/done and we can then focus on #8 and ghdl#3. What do you think?

I am not. What do you mean by 'received in vhdl' ?

impure function c_getStruct return c_struct_ptr is where c_struct_ptr is an access type, and the c function returns a struct*. What I mean to highlight is that the returned struct* does not get passed by reference to produce a struct** or access access.... Clearer?

I think that VHDL/GHDL is not aware of the prototype used in C. Both struct* and struct** are valid because VHDL will dereference the pointer once and obtain the VHDL record type. Independently of the prototype in C, the value of something which is a reference to a struct/record is the same. I believe this is the same case as c_intArr above.

So there is an asymmetry: records from VHDL -> C are passed by reference, structs from C -> VHDL are passed by value (so C must pass a struct*, which is handed over by value).

I think that, even if C passes a struct*, which you define as being handed over by value, VHDL/GHDL can still change the content as it had been passed by reference, isn't it? I mean that, AFAIU, the difference for a function receiving a value or a reference in C, is that the latter modifies the content in the original location and the former does not. I think that this is what we need to focus on: when can VHDL/C modify a value that they received and when they need to use it as "read-only".

Copy link
Owner

@umarcor umarcor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, LGTM, it's nice and clean!

However, I had to read the sources a couple of times to understand how it fits with other examples in 'sharedvar'. In the 'parent', 3-4 different methods are used. Here, it seems that one is used only. That's ok, because we don't want to duplicate what we already explained. Nonetheless, we need to make it explicit. For example:

From the different methods explained above, here THAT one is used. This is because it allows the same description to be used with either VHDL 1993 or VHDL 2008.

.gitignore Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
doc/vhpidirect/examples/quickstart.rst Show resolved Hide resolved
@@ -0,0 +1,47 @@
#include <stdio.h>

static const char HDL_LOGIC_CHAR[] = { 'U', 'X', '0', '1', 'Z', 'W', 'L', 'H', '-'};
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At this point of the guide, logic vectors are not introduced yet. There are multiple options:

  • Leave it here, but add a note to the docs making it explicit that HDL_LOGIC_CHAR and HDL_LOGIC_STATES are explained in arrays/logicvector.
  • Don't use std_logic types here, but stick to constrained strings, character, integer, real, time, etc.
  • Move this as a subexample of logicvector. So, instead of "proposing" the readers to jump forward from the quick start guide, we suggest them to jump back from the advanced example.
    • If we did this, we might want to consider renaming 'arrays' to 'arrays and structs'.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this as a subexample of logicvector.

I think that it should come after the arrays section.. But what we are really getting around to is that "arrays" is the start of explaining sharing more complicated variables. I think ultimately we will have a section on strings too, maybe move the logic-vector to enums, and then have records. So I'd starting to feel like arrays should be a under sharedvar, and these other sections too.

Thoughts?

I think for right now, we can make that reorganisation as another pull yeah? And I will just clean up the example. And just go for adding a note that the logicvector example explains things further.

vhpidirect/quickstart/sharedvar/shrecord/caux.c Outdated Show resolved Hide resolved
variable struc: c_struct := (
logic_bit => 'X',
logic_vec => ('H', '0', 'H', '0', 'X', '1', 'X', '1'),
int => 144
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra tabs

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still showing some extra tabs between int and =>. Or is it my browser cache?

@radonnachie
Copy link
Author

I had to read the sources a couple of times to understand how it fits with other examples in 'sharedvar'

Yeah, as discussed in one of your comments, this isn't exactly the best place for this example. It probably should have gotten its own 'records' page. But (again, as discussed) I think a good reorganisation will come around soon.

@umarcor
Copy link
Owner

umarcor commented May 13, 2020

Absolutely. Just enhance the description slightly and we can merge this.

Regarding the reorganisation, I'm about to push a PR that shows how to execute Python callbacks from VHDL (it's much nicer than I had though :D). I.e., we can use numpy and matplotlib in VHDL testbenches! This example is related to both 'shghdl' and 'arrays/matrices/vga'. Hence, I had the same thought about "the start of explaining more complicated variables". As you said, let's think about it and we'll discuss it soon.

@radonnachie
Copy link
Author

the strategy that GHDL uses to return a complex type by value, is to add the facade of a pointer to the type.

Oh yes, I do recall seeing the void func(complex_t *returnArg) declarations. I imagine the records are the perfect place to showcase that. Your above explanation will do nicely! Thanks.

Hence, let's have this PR reviewed/done and we can then focus on #8 and ghdl#3

Agreed, this value/ref thing was just for my curiosity. and for documentation sooner or later.

even if C passes a struct*, which you define as being handed over by value, VHDL/GHDL can still change the content as it had been passed by reference

💯 I was trying to say the the pointer gets handed over by value, ie an address gets handed over. Which is why I was talking about it as an asymmetry - ghdl does not pass c-function returns by reference, but by value. For the complex type, you have to return a ptr, an address, and ghdl passes that address by value to vhdl, which parses it as an access type.

I saw the reverse too, where a constant record in vhdl was passed by reference to C, and C could change the fields of the constant record.

@umarcor
Copy link
Owner

umarcor commented May 13, 2020

Oh yes, I do recall seeing the void func(complex_t *returnArg) declarations. I imagine the records are the perfect place to showcase that. Your above explanation will do nicely! Thanks.

I will be so glad if you help me put this nicely in the docs. In this case, it's not because of the language barrier, but because of the C terminology which I'm not sure to always use properly.

Hence, let's have this PR reviewed/done and we can then focus on #8 and ghdl#3

Agreed, this value/ref thing was just for my curiosity. and for documentation sooner or later.

In fact, I think it's good that we are starting to discuss some of this issues before diving into #8. I believe that they require some time to wrap your head around them. This is also the reason for delaying #8 ~1 month since you proposed it, and forcing ourselves to understand the details of each specific case first. Hope you understand the strategy.

💯 I was trying to say the the pointer gets handed over by value, ie an address gets handed over. Which is why I was talking about it as an asymmetry - ghdl does not pass c-function returns by reference, but by value. For the complex type, you have to return a ptr, an address, and ghdl passes that address by value to vhdl, which parses it as an access type.

I saw the reverse too, where a constant record in vhdl was passed by reference to C, and C could change the fields of the constant record.

We are riding really close to the end of the cliff. The view is so nice, but sometimes sea and land get angry :D. In the end, we are using some convention to pass pointers and values between VHDL and C; yet, each language has a completely different internal behaviour and knowledge of the context. I.e., VHDL/GHDL cannot prevent C from tinkering with arbitrary content in arbitrary addresses, because both are siblings under the same roof (application memory space). Each of them can limit their children, but not nephews. But... this is the fun part, isn't it?

integer's in previous examples. The :ref:`COSIM:VHPIDIRECT:Examples:arrays:logicvectors` example fully explains this variable.

As mentioned in :ref:`Restrictions_on_foreign_declarations`, records are passed by reference. This means that the functions
in C recieve and return ``struct *``. In VHDL, the VHPIDIRECT subprograms will return record access types, while those with
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, the PR is ready. I don't think it is worth to fix the single issue with the tabs. However, please let me keep this on hold until I can run a couple of tests to fully understand this and the note below.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure. How I tested it was making any combination of these 2 changes: (struct* c_func... -> struct c_func...) and (function c_getStruct() returns record_access to function c_getStruct() returns record)... only the committed 1 of the 4 combinations results in a working example.

Let me know if you find anything different. Perhaps without going into GHDL's trick of a hidden argument being what it returns....

@umarcor
Copy link
Owner

umarcor commented Jul 14, 2020

@RocketRoss, in the end, I was quite busy and I didn't have the time to run the tests I wanted. My apologies 😞 . Anyway, I merged this along with other additions and it is already pushed (ghdl@a519fd9). I fixed some typo, pasto and #17 (comment), but apart from that, it is as you wrote it.

Anyway, if required, we will revisit the wording of https://ghdl.github.io/ghdl-cosim/vhpidirect/examples/quickstart.html#shrecord when I have time to run the tests I wanted.


Regarding latest additions, apart from changing the style slightly, examples https://ghdl.github.io/ghdl-cosim/vhpidirect/examples/quickstart.html#command-line-arguments and https://ghdl.github.io/ghdl-cosim/vhpidirect/examples/shared.html#pycb were added; and section https://ghdl.github.io/ghdl-cosim/ci.html. We have +20 examples already 😄

I believe that the addition of CLI examples might allow to simplify/focus #5 and/or #11.

About the example to execute Python code from VHDL, I hope you find it fun/useful.

Overall, I'd be glad if you could review the new content (whenever you have some time).

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 this pull request may close these issues.

None yet

3 participants