Subject: RE: [sv-cc] Comments w.r.t. Section 29, SystemVerilog Data Read API
From: Bassam Tabbara (bassam@novas.com)
Date: Thu Jan 08 2004 - 16:45:33 PST
Thanks Michael for the input. I will update with your feedback after the
vote.
-Bassam.
-- Dr. Bassam Tabbara Technical Manager, R&D Novas Software, Inc.http://www.novas.com <http://www.novas.com/> (408) 467-7893
-----Original Message----- From: owner-sv-cc@eda.org [mailto:owner-sv-cc@eda.org] On Behalf Of Michael Rohleder Sent: Wednesday, January 07, 2004 11:34 AM To: SystemVerilog CC DWG Subject: [sv-cc] Comments w.r.t. Section 29, SystemVerilog Data Read API
Hi all,
here are my comments w.r.t. the latest document from Bassam (sent by http://www.eda.org/sv-cc/hm/1679.html), the document is named data_read_vpi-12-29.pdf.
Don't be shocked by the mass of write-up I did, I added some suggestions and explanations, which makes it rather lengthy, but this might save some time. In general I like it very much and believe Bassam and the team has done a great job here. Thanks a lot to all of you.
Best regards, -Michael
(Main) comments: - Table 29-2: New Reader VPI routines, 3rd row, column "To" (pg 9) the last sentence in brackets ("except in the case of calling the routines in the default library") is misleading, since it suggests the existance of a default library (what is this?) as well as it looks like an exception to the access indirection through the reader_extension_ptr for this library. My understanding is that a) there is no default library and b) only 'native' VPI functions can be directly called. Suggest changing the sentence "All following VPI routines are called by indirection through the returned pointer (except in the case of calling the routines in the default library)" to "All VPI routines defined by the reader extension library are to be called by indirection through the returned pointer; only {native/built-in/standard/or whatever} VPI routines can be called directly"
- sub-bullet a) of bullet 1) on page 10 saying "Name of the reader library to be used specified as a character string. A NULL entry means that the default library (i.e. tool (e.g. simulator) the application is running under) is used.". Here is again the default library, where I simply don't understand whether we request the existance of such a library. Also, it is not mentioned what is the 'name of the reader library'. My understanding until now is that this is either a static or shared library implementing the reader extension. It could therefore be a) a pathname to this file or b) just the filename [assuming some search rules for libraries] with or without an extension. There are some problems with this: * the requirement to specify an extension immediately means the corresponding code is platform dependent (Urrrgh) * the requirement to specify a full pathname might result in non portable code + might implicitely introduce again platform dependencies (this can be easily a killer for reuse ...) Suggest to change the sentence from "Name of the reader library to be used specified as a character string. A NULL entry means that the default library (i.e. tool (e.g. simulator) the application is running under) is used." to "Name of the reader library to be used specified as a character string. This is either a full pathname to this library or the single filename (without path information) of this library, assuming a vendor specific way of defining the location of such a library; the latter method is more portable and therefore recommended. Neither the full pathname, nor the single filename shall include an extension, the name of the library must be unique and the appropriate extension for the actual platform should be provided by the application loading this library." I am still unsure about the usage/definition of a "default library" and its implication. Suggest to remove anything relating to such a library. Please review also the sample code 29.7.4.1 where this is used ... Also suggest to modify 29.10.2 (page 23) accordingly
- sub-bullet b) of bullet 1) on page 10 saying "Database file with stored data in case of vpiAccessPostProcess mode (...)" Here are two problems a) I am confused whether I just need to specify a name here for vpiAccessPostProcess, the description of the Access mode below suggests the usage of a 'scratch file' for vpiAccessInteractive. We should clearly state when it is required to provide a name here and when not. No suggestion, sorry b) My understanding is that this is a logical database name, this should be reflected and explained here, so it is clear that this is not neccessarily the name of a file in the file system. Suggest to rephrase to "Name of the logical database holding the stored data in case of <list the required access modes here>; a NULL can be used in case of <list optional access modes>. This is the name of a database, not the name of a file in the file system. It is implementation dependent whether there is any relationship to an actual on-disk object and the provided name." There are several references to a "data file" in the text below, suggest to change these to "database", similarly in 29.7.5 (sample code) and 29.9 (example) Also suggest to modify 29.10.2 (page 23) accordingly
- suggest to call vpi_read_close() for _all_ access modes. Suggest to change the second block after the example on how to call reader_extension_ptr->vpi_get(...) in the second half of page 10 accordingly. There might be cleanup actions required for other access modes as well. We should permit also implementations requiring this ...
- 29.9 second page 20: the content of the s_vpi_extension structure is underspecified. There should be at least a statement on how the functions within this structure are to be ordered. It looks to me like it is intended to be exactly in the order of VPI functions defined in the IEEE 1364 standard (vpi_chk_error is the first, vpi_printf is the last such function), but it would be not a bad idea to state this by a sentence. We must be rather exact here...
- 29.9 two sections before the big example code at the end of this section. The sentence "Note that, as stated earlier ..." Suggest to reword "default routine implementation" to "built-in routine implementation"
- It might be good to state in the description vpi_load_extension() on page 23, that the generic definition of this function permits later, non reader specific extension; all other arguments are specific and defined for the reader extension.
(Very) minor comments - GENERAL I am unsure how common the abbreviation VC for value change is. I am used to VC for virtual component, so this stuff could be rather confusing. You might want to consider a 'search and replace for VC to value change'...
- 29.1.1 Requirements (pg 1) The section starts with "SystemVerilog adds several design and verification constructs including:", but the following list just mentions data type extensions. I think it would be good to change "- User defined data types" to "- User defined data types and corresponding methods" and "- Test bench data types and facilities" to "- Data types and facilities enhancing the creation and functionality of testbenches" to make the first sentence a little more true.
- 29.2.1, 29.2.2.1, and 29.2.2.2.1 I would leave some holes in the #defines for the types (e.g. by starting a new block at a new 10 number), to leave some space for later extensions. Suggest to start vpiIsLoaded at 810, vpiAccessLimitedInteractive at 820, vpiMember at 830, and vpiMinTime at 840.
- 29.7.1 section after the example on how to call vpi_get (middle of page 10) I think it should say "is the reader library pointer returned by" instead of "is the reader library pointer returned after"
- 29.7.5 Sample code: in the call of vpi_load_extension() the last two arguments are switched ...
- 29.9 3rd example showing the use of vpi_load_extension(): A perfect check would first check the struct_size to be greater or equal than the expected value (to make sure we are on the save side before dereferencing), and then check extension_version and extension_name to be non-NULL before dereferencing it ... Then we might want to check the version to be greater or equal to 1 (or have a switch statement processing possible further versions ...), but this is of second order ...
--
NOTE: The content of this message may contain personal views which are not neccessarily the views of Motorola, unless specifically stated. ___________________________________________________ | | _ | Michael Rohleder Tel: +49-89-92103-259 | _ / )| Software Technologist Fax: +49-89-92103-680 |( \ / / | Motorola, Semiconductor Products, System Design | \ \ _( (_ | _ Schatzbogen 7, D-81829 Munich, Germany _ | _) )_ (((\ \>|_/ > < \_|</ /))) (\\\\ \_/ / mailto:Michael.Rohleder@motorola.com \ \_/ ////) \ /_______________________________________________\ / \ _/ \_ / / / \ \
The information contained in this email has been classified as: Motorola General Business Information (x) Motorola Internal Use Only ( ) Motorola Confidential Proprietary ( )
*** This note may contain Motorola Confidential Proprietary or Motorola Internal Use Only Information and is intended to be reviewed by only the individual or organization named above. If you are not the intended recipient or an authorized representative of the intended recipient, you are hereby notified that any review, dissemination or copying of this email and its attachments, if any, or the information contained herein is prohibited. If you have received this email in error, please immediately notify the sender by return email and delete this email from your system. Thank you! ***
This archive was generated by hypermail 2b28 : Thu Jan 08 2004 - 16:46:50 PST