RE: Some review feedback on the DPI chapter

Brian,

I have copied the pervious version I sent to the template file and
updated this per comments sent by John and Per. Please post this version
3 in the SCE-MI 2.0 DPI section on the web. I assume that version 3 will
follow soon given the number of issues we need to discuss, but we should
at least use a clean copy as the basis for the next review/update.

Per,

Here are my comments.

>-----Original Message-----
>From: owner-itc@eda.org [mailto:owner-itc@eda.org] On Behalf Of Per
Bojsen
>Sent: Wednesday, October 04, 2006 11:56 AM
>To: itc@eda.org
>Subject: 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.
[Shabtay] Not in the version I originally sent. I think you were still
looking at the current corrupted version. 
>
>* 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.
[Shabtay] I reviewed the IM several times. My language addresses Russ
requirement but leaves the flexibility to vendors to provide variety of
options. But if you want to define a common default, we could define
that X and z will be coerced to 0 and 1 respectively.
>
>Furthermore, I think it might be a good idea to clarify that the
>coercion only happens in the C->HDL direction.
[Shabtay] Agree.
>
>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.
[Shabtay] I added some language to 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'.
[Shabtay] OK
>
>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.
[Shabtay] Let's discuss this as this is significant change.
>
>The informational note about PLI can/should be dropped.
[Shabtay] Agree.
>
>The SystemVerilog DPI context handling example can go in the
non-normative
>note or informational section with the above first example.
[Shabtay] Let's also discuss this as this is significant change.
>
>* Section 9 (Ve)rilog native syntax used ...
>
>I think this is another instance of a section that was intended to be
>complete deleted.
[Shabtay] you are looking at the wrong version.
>
>* 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.
[Shabtay] I suggest that we revisit this for SCE-MI 2.1. For now this is
as far as I can go.
>
>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.
[Shabtay] See my reply to John. 
>
>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'.
[Shabtay] Agree. Modified.
>
>* 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?
[Shabtay] John. You did. Any comments?
>
>The example should be a non-normative note.
[Shabtay] 
>
>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.
[Shabtay] I agree. I changed all the other text to informative so we can
discuss this before I remove it.
>
>* 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.
[Shabtay] The first paragraph caused confusion as my intent was to
mention that recursion is not allowed. But thinking about this, this is
already defined by DPI. As to the rest of the section, the intent is to
define minimum level of nesting in SCEMI 2 but not prohibit vendors who
wish to provide more than one level.
>
>* 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.
[Shabtay] I'll need more time to review this.
>
>* 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.
[Shabtay] This issue deserves a discussion. I don't think we can extend
DPI support to any unrestricted threaded application and to SystemC that
is not running on a shared simulation kernel. Even if the above can be
done, it needs to be tested before such language is inserted to the
SCEMI 2 spec. So I am afraid that unless we narrow the scope to SystemC
running on a shared simulation kernel we won't be able to extend the DPI
use model beyond what's being defined in the SV LRM. 
>
>Enjoy,
>Per
>