Some review feedback on the DPI chapter

Hi,

here is some feedback on Shabtay's chapter, the DPI chapter.  I don't
trust the section numbering given the churn we've been having with
automatic numbering, track changes, etc., so I'll try to add section
headers as well to make it easier to find what the comment relates to.

* Section 1.1.1.3 (Str)ing arguments in Verilog 2001+

According to IM201 we decided to not support string types.  As far
as I can tell you intended to delete this whole section but a
sentence got left.

* Section 1.1.1.4 4-State logic types [IM212]

Under the coercion bullet it is stated that X and Z will be coerced
to 0 and/or 1 using an implementation specific mapping.  I think it
makes sense to specify in the standard what that mapping should be.
For instance, it could be defined to be equivalent to what you would
get in Verilog by setting the B value/vector to 0.  The rationale
for specifying this is to ensure you will get identical behavior on
different (non-4-state capable) platforms.

Furthermore, I think it might be a good idea to clarify that the
coercion only happens in the C->HDL direction.

Finally, I'd like to request that a non-normative mode be added that
recommends that implementations provide a debug mode that would flag
when coercion happens.  This would be very useful for users to quickly
debug potential issues caused by coercion.  After all, coercion is
usually not what one wants to happen as a matter of use model.  Rather,
a proper use model is to avoid sending X and Z to the HDL side at all.
Perhaps a use model note should be added to that effect either here
or in the informative section.

* Section 1.1.1.5 Context handling

This section uses the term `ANSI C free function'.  I think I know
what this means but it is not an obvious term.  I suggest we drop
the word `free' and also `ANSI' and just call it `C function'.

Actually that whole first paragraph with the example feels more like
background, non-normative text that should perhaps be moved to a note
or the intro sections.  From a spec point of view, all that matters
is that we support the SystemVerilg svGetScope() and svGetUserData()
functions for context handling.

The informational note about PLI can/should be dropped.

The SystemVerilog DPI context handling example can go in the non-normative
note or informational section with the above first example.

* Section 9 (Ve)rilog native syntax used ...

I think this is another instance of a section that was intended to be
complete deleted.

* Section 10 Time consuming tasks [IM232]

First off I think it is unfortunate that the proposed text states
that time consuming tasks are not supported.  Time consuming tasks could
be quite useful in modeling transactions.  For instance, this would
allow the SCE-MI 1.x style of handshaking with a transactor by
sending the command to a input port and then waiting for the result of
the transaction to come back on some output port to be abstracted in
a single function call, e.g.,

   // C side of test.
   SendTransaction(<args here>);

   // SendTransaction() call returns when transaction has comleted.
   // Send another transaction.
   SendTransaction(<args here>);

If time consuming tasks are not allowed you have to use at least
two function calls to achieve the equivalent of the above.  One
call would send the transaction and another would either poll for
completion (C to HDL call) or would information the C side of
completion (HDL to C call).

Being able to call time consuming tasks is a feature that would make
the addition of DPI support to SCE-MI something radically better than
SCE-MI 1.1.

Secondly, I agree with John that limiting the support to functions
is too restrictive even if time consuming tasks ends up not being
supported.  We should support at least 0-time tasks and ideally
non-0 time tasks as well.

Finally, the heading of the section is `Time consuming tasks' but
in reality it talks about functions and non-time consuming tasks as
well.  Perhaps the title should be simply `Functions and tasks'.

* Section 11 Support for multiple messages in 0-time

The informational paragraph in blue is misleading.  If not read
carefully it could be interpreted as stating that SCE-MI 1.x does
not support sending multiple messages in the same timestep.  This
is clearly wrong as you can send any number of messages while the
controlled clocks are stopped.  The DPI mechanism of doing multiple
calls in the same timestep is exactly equivalent to this.  I
suggest the informational text gets removed.  Perhaps we can replace
with another message that highlights the similarity between making
multiple DPI calls in one time step and sending multiple SCE-MI
messages.  Who originally wrote this blue section?

The example should be a non-normative note.

I suggest that the first paragraph gets rephrased.  It currently
reads as part of a sales pitch for why DPI is better than SCE-MI 1.x.
Essentially, it is trying to explain that the DPI subset does not
lose the SCE-MI 1.x capability of sending multiple messages per
timestep.  In this light, the blue text is even more misleading
by the way :-)

Here is an attempt at rephrasing the section:

   11. Imported function calls in same timestep

   DPI places no restrictions on the number of imported function
   calls made in the same block of code without intervening time
   advancement.

Perhaps this is too terse, but I think it is better than the
existing text.

* 12 Rules for DPI function call nesting and recursion [Re: IM202]

The second paragraph can be removed as it is implied by the first.
The third and fourth paragraphs contradict the first paragraph.
Either the first paragraph has to be changed to say:

   The result of calling an imported DPI function from an exported
   DPI function/task is undefined.  Imported DPI functions may,
   however, call exported DPI functions/tasks.

Or paragraphs 3 and 4 should be removed.  Actually, with this
change in wording we can still paragraphs 2-4 regardless as
the paragraph affords the implementation leeway to implement
multiple levels of nesting under the `undefined behavior'
umbrella.

 From an implementation point of view the question is whether
doing the error checking required by the original wording of
the first paragraph is hard to implement.  With the original
wording a standard SystemVerilog simulator would not be a
compliant implementation of SCE-MI 2.0 as far as DPI goes, but
that is probably not the only area where there are problems
anyway.

Using the `undefined behavior' wording above is a departure
from SCE-MI 1.1 where we worked hard to specifiy things in a
way where there was no built in leeway for implementations
to choose what to do.  I am on the fence as to whether I
prefer the `error' wording or the `undefined' wording.

* 13 DPI utility function supported by SCE-MI 2 [missing IM]

My email also mentioned that we need to include some type
definitions that we support:

   svScope
   svBit
   svBitVecVal

With the 4-state support we also need to support the 4-state
types:

   svLogic
   svLogicVecVal

There are also some standard #defines we need to support:

   #define sv_0 0
   #define sv_1 1
   #define sv_z 2 /* representation of 4-st scalar z */
   #define sv_x 3 /* representation of 4-st scalar x */

While we are added we should probably also support the following
trivial helper macros:

   /* Number of chunks required to represent the given width packed array */
   #define SV_PACKED_DATA_NELEMS(WIDTH) (((WIDTH) + 31) >> 5)
   /*
    * Because the contents of the unused bits is undetermined,
    * the following macros can be handy.
    */
   #define SV_MASK(N) (~(-1 << (N)))
   #define SV_GET_UNSIGNED_BITS(VALUE, N) \
       ((N) == 32 ? (VALUE) : ((VALUE) & SV_MASK(N)))
   #define SV_GET_SIGNED_BITS(VALUE, N) \
       ((N) == 32 ? (VALUE) : \
       (((VALUE) & (1 << (N))) ? ((VALUE) | ~SV_MASK(N)) : ((VALUE) & SV_MASK(N))))

As for functions, supporting 4-state logic values adds the need for the
following functions:

   svLogic svGetBitselLogic(const svLogicVecVal* s, int i);
   void svPutBitselLogic(svLogicVecVal* d, int i, svLogic s);
   void svGetPartselLogic(svLogicVecVal* d, const svLogicVecVal* s, int i, int w);
   void svPutPartselLogic(svLogicVecVal* d, const svLogicVecVal s, int i, int w);

I may have missed some as I did not have time to go through P1800 appendix
F thoroughly.

Note, the main motivation for supporting all these functions and #defines
is twofold:

   1) Allow as broad a range of user code to just work without editing as
      possible and reasonable.  This is the same motivation that was used when
      adding support for 4-state logic.
   2) The functions and macros are trivial to implement (most of them anyway).

I originally listed several functions that I had some questions about:

   const char *svGetNameFromScope(const svScope scope)
   svScope svGetScopeFromName(const char *scopeName)

In my original email I stated the above were needed by Jason's proposal
for wrapping the HDL side pipes API in a module.  Is this still true?
I guess you would use these function to refer to the pipes by name.  This
seems very reminiscent of SCE-MI 1.1 port binding and should be implementable
by a similar underlying mechanism.

   int svGetCallerInfo(char **fileName, int *lineNumber)

This one I have my doubts about.  In the original email I suggest we could
have it return always FALSE to indicate non-success of getting the filename
and line number information.  This function would be a challenge to implement.
I suggest we either drop it or specify that it returns always FALSE in
SCE-MI 2.0.  However, that is perhaps not so useful.  While it allows code
using it to compile it could still cause code to break and definitely will
can cause the code to change behavior.

   const char *svDpiVersion(void)

For this one, the main issue is what to return as the DPI version?  If we
returned the same as SystemVerilog does we would be lying sort of since
we are not a compliant DPI implementation (we support only a subset after
all).  Again, there is the issue of code breaking if code has somehow been
written to depend on the result of this function.  Coding guidelines should
as a minimum recommend that the return value only be used in printing
and logging.

* 14 Calling DPI exported functions outside an imported context function [Re: IM219]

The reference to P1800 that states that calling exported tasks/functions
from non-context imported DPI functions is undefined is Section 26.4.3
3rd paragraph.

This section makes extensive reference to SystemC.  I don't think it is
appropriate to refer to SystemC in this spec.  On the software side, the
spec should refer only to C as that is the lowest common denominator we
have agreed on.  From this behavior of environments such as SystemC
must be inferred, but should not be explicitly stated except perhaps in
appendices, non-normative notes, or in the introductory section.

Also, when you talk about SystemC you implicitly imply that the SystemC
code is running on a shared simulation kernel with the SystemVerilog
code (or in cosim mode with the emulator).  But you can also run
SystemC in the OSCI simulator which is not coupled to the HDL simulator
and for which the elaboration/construction phase comments do not apply.
For instance, OSCI can have its elaboration phase before or after
the simulator's elaboration phase.  The simulation times may not be
synched at all.

Also, the last paragraph seems to prohibit calling exported functions
from any threaded application in any case.  It also contradicts
the second paragraph, by the way.  Besides this being a tighter restriction
than the one in Section 12 (where exported functions may be called from
imported functions) it also refers to threaded environments.  I thought we
had agreed to keep threading separate and independent from SCE-MI 2.0.

* 15 Calling DPI utility functions outside imported context functions

I have the same issues with the explicit reference to SystemC in this
section as in the previous one.

This section also depends on what we decide to do with the resolution
of our issue with the SV committee.

That's it for now.

Enjoy,
Per