Subject: RE: [sv-cc] Fwd: READ api issues
From: Francoise Martinolle (fm@cadence.com)
Date: Fri Jan 09 2004 - 21:17:45 PST
Comments starting witn FM;
At 12:47 PM 1/9/2004 -0800, Bassam Tabbara wrote:
>Francoise, thx a lot for the feedback. I will attempt to skim and offer an
>opinion (since Swapnajit had requested this), all in all I do not think
>there is substantial disagreement here, and I think some editorial fixes
>can help clarify/address the issues you raise. Looking forward to getting
>all the team's support in the vote, and the final polishing that must be
>done to get the READ VPI in LRM form.
>
>** I honestly think these are either cosmetic/philosophical in nature, or
>are valid editorial fixes. I think in some cases, I have to choose in
>favor of the Novas tremendous experience in this serving different and
>varied concerns from partner and customer alike (so yeah forgive the
>"fluff" in a couple of cases but it's worth it), and my true attempt at
>munging all of the different uses and applications of our API (and the
>wealth of API routines ...), and I've done my best to present that in VPI
>form in the proposal, in some cases using my best judgement tempered by
>all the excellent input from all in keeping this in VPI form, so I frowned
>on ideas that would move this away from a VPI style, I think we all like
>that accomplishment so let's keep it. I sincerely appreciate all of your
>efforts to help in this regard and correct the missteps or things I
>dropped in all the revs so far. I've taken to heart Michael's comments and
>Francoise's comments (before now and the ones below).
>
>** I only mean to help in the discussion below (and **only because
>Swapnajit requested this**, I think people already know most of these) so
>please take it at that, forgive my lack of detail in a couple of comments
>(only because I've seen this so many times, but I know Francoise is
>summarizing everything).
>
>-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
>Francoise Martinolle
>Sent: Friday, January 09, 2004 8:09 AM
>To: sv-cc@eda.org
>Subject: [sv-cc] Fwd: READ api issues
>
>>My previous email bounced.
>
>
>
>
>
>
>>The Cadence VPI/database experts and I have reviewed the latest proposal
>>and we are still not satisfied with the final specification. We have
>>brought some of these issues in the past and we still think that these
>>issues are important and should be solved.
>>
>>When accessing data from multiple databases, the user application must
>>keep track of which handles comes from where. Instead it should be
>>possible to query which database a handle originated from. Our preference
>>would be to create a database handle when opening a database and allow to
>>obtain the database handle back from any other handle obtained from the
>>database. The current proposal specifies a new property (vpiInExtension)
>>to determine if a handle was obtained by a specific access to a database;
>>this does not work at all in the following two scenarios:
>>[BT] The heavy cost of extra complication with the addition of that extra
>>"db" layer, and they are only for cases of "postprocess" mode is not
>>worth it. In some sense they break the "advantage" built into the
>>proposal of "treating similar things in the interactive/postprocess modes
>>the same", and "keeping things simple". For all practical concerns you
>>already have a **top scope handle** even when you have multiple scopes
>>they are independent so you have a couple of handles, or even potentially
>>you can always have a **virtual top scope** in the db storage, so have
>>the extra db type floating around and so on in all cases is NOT
>>worthwhile in my judgement. So you also have one handle in that case, but
>>really a couple of handles is not that big of a deal (we;re talking VPI ...).
>>[BT] Adding the db creates a lot of indirection to everything, you cannot
>>then create a handle from a scope without passing a db pointer,to really
>>do this as you suggest, if you mean a "virtual top" that's a minor thing
>>(but not worth the typing ...), if you mean really using the db handle in
>>all handle creation, that means this READ API will no longer look like
>>VPI !!! I cannot then call vpi_handle() to get the trvsHndls and so on,
>>and that's is NOT what I have in mind, and I think a lot of the members
>>would not be happy with that approach...
FM: I only mean that from any object that had obtained you can always ask
which db it came from. The dbHandle acts much like a scope/module. It is a
new one to one method. You don't have to ask about the db handle unless you
need it. You can keep track of the handles yourself but it may become
easier for an application to sometimes be able to get back the dbhandle.
>> a) I have a handle from a different extension, and that handle
>> implementation is completely unknown to my inferface function vpi_get,
>>[BT] you get a FALSE.
FM: Joao explained to me that you have to know that your interface created
handles in a given address space in order to do this. It would work, I think.
>> b) I have two databases for which I use the same READ api to access
>> it. Two handles coming from each databases would always have this
>> property TRUE. I can't tell if these handles belong to a different
>> database because I have no database handle to compare it to!
>>[BT] Hmm, you can always compare the handles! This situation is no
>>different than having multiple handles in any VPI ...
>>[BT] I think in most cases having a top scope handle is sufficient. In
>>other cases you would have a couple of (top) scope handles, or you can
>>choose to add in the db a "virtual top" (aka root), to clarify the
>>situation in case there are many top scopes (i.e. forest) in a single
>>database. The "InExtension" is really for the READ api, yes. I think a
>>database handle (aka root) adds another layer so for common case it adds
>>complication. I do not think this is a worthwhile complication worthy of
>>addition at this time. For all intents and purposes keeping things simple
>>serves users much more than adding complication.
>>
>>The current proposal does not separate the data accessed via a simulator
>>from the data accessed from the data base management software. We would
>>prefer to have different vpiTypes for handles obtained from a database,
>>the reason is that the representation of a vpiNet in a database is
>>completely different from a vpiNet obtained from the simulator. With the
>>inability to tell from where it obtained the handle, an application has a
>>hard time keeping track of the handles.
>>Our preferred approach is to have a vpiDbObject type to represent a
>>database object, there would be an additional property (vpiOrigType) to
>>indicate the type of the object it originated from in the design. For
>>example a vpiNet object in the design would have a corresponding
>>vpiDbObject in the database domain. The only correspondence which would
>>exist between the simulation object and the database object would be that
>>they have the same vpiName and vpiFullname.
FM: after discussion with Joao, I kind of agree with the approach taken of
using the VPI information model even for handles pointing to objects from
the database.
>>[BT] Again the intent is to be simple and allow flexibility. This goes
>>hand in hand with the above if there is a separate dbobject than this is
>>ok. But in my opinion a separate db object really breaks the intent of
>>the proposal to minimize different teatment of traverse objects
>>**regardless of where they came from** whether simulator or db or what
>>have you ... I do not think this simplicity is a burden it rather makes
>>things neat, and there is no confusion users already keep track of
>>handles of scopes and the like ... the extra virtual top is just an
>>additional level of indirection, with no practical value.
>>In the scheme proposed to have and use more than one set of VPI routines
>>at the same time the specification of the vpi_load_extension() is
>>incomplete. In fact the vpi_load_extension() is the only routine which is
>>NOT part of the READ API library. This routine should be implemented by
>>the application or tool which needs to invoke the READ API and needs to
>>be provided as a separate object or library. There are some conventions
>>on the shared library name and bootstrap function for the READ API which
>>need to be explicitly stated.
>>
>>[BT] Hmmm, to the extent that this overlaps with Michael's comments about
>>clarifications for the ***READ API concerns** I agree, we have these
>>editorials. As for conventions, and bootstrapping and the like these are
>>beyond the scope of this chapter ....
FM: It should at least be said that this function implementation is
supposed to be provided separately and executed prior using the READ
API. I think that delivery of a shared library and integration should be
part of a standard in order to enable full portablity of that library. A
user which uses multiple vendor has always a problem trying to integrate
the same API with different simulators. We should all agree on a standard
mechanism.
>>The API is not general enough to also be applicable to writing the data
>>base, which is a desirable trait. If a function to open a database was
>>provided with the capability to indicate the access (read, write...) to
>>the database, that would be preferable to an implicit data base open with
>>vpi_read_load().
>>
>>[BT] This is incorrect in my opinion. The trait is there, only this is
>>the READ API.
FM: I think that what I really meant there is to generalize the functions a
bit so that they can be easily extended to also be used in the write case.
For example having a function to open a database such as;
vpi_db_open(filename, access, flags) where access can be read or write.
>>
>>Some of the extensions are not in line with the spirit of VPI:
>> provide basic building blocks
>> provide a very few number of general functions.
>>Specifically:
>>Some of the functions added could have been done with existing VPI
>>functions.
>>Ex: vpi_trvs_get_time() can be accomplished with vpi_get_time()
>> vpi_goto() can be accomplished with vpi_control() and vpi_filter()
>>
>>[BT] This is also incorrect. **ALL** the extensions are done to the
>>existing VPI when applicable, and you can do **everything** using that
>>set of API. An **additional** set is provided when the corresponding VPI
>>cannot handle the extension for backward compatability (e.g. vpi_goto()
>>has some additional function on vpi_control() but you can use
>>vpi_control() to do the same function).
FM: If vpi_goto can do what vpi_control can do and more, then we should
only specify vpi_goto. vpi_get_time can be used to do what
vpi_trvs_get_time can do . You can use the type field in the time structure
to indicate the time : vpiNextVC, vpiPrevVC... vpi_get_time takes a
reference handle which can be a trvsobj.
vpi_goto should take a collection handle or a trvsobj and a s_vpi_time
structure and return a new collection or trvsobj.
>>Some functions added have slight variations which could have been
>>implemented with existing functions. ex: vpi_load_init_create() could
>>have been implemented by creating a collection of all objects within a scope:
>> Ex: colH = vpi_create(vpiCollectionK, 0, scopeH, vpiObjDecls) ;
>> vpiObjDecls is a new iteration which will return all variables, regs,
>> nets in a scope.
>>
>>[BT] This is a philosophical issue. Whether a collection is a mix of
>>things, or there are some special types of collections. I chose the
>>latter, the former is there for later extension. It seems reasonable as
>>an architectural design decision of the proposal to have a special
>>vpiTrvsCollection type.
FM: I think it would be better to have a subtype of collection which can
indicate what it contains, but keep the general collection as the only vpiType.
>>There are many variations of functions : vpi_read_load(),
>>vpi_load_init(), vpi_load_init_create() with no distinct advantage.
>>[BT] The advantage of multiple methods is flexibility. An application can
>>chose what is best for it. A complex routine need not be called if I do
>>not need it.
FM: The functions proposed only accomplish what you can do with a few vpi
function calls. VPI has been designed to provide building blocks not
convenience functions.
>>The application needs to concern itself with whether the data it is
>>accessing is loaded into memory. This instead should be an implementation
>>issue. Whether or not the data is loaded should be transparent to the
>>application. When accessing the trvsObj handle with vpi_handle(), the
>>data should be loaded if not already loaded. There should not be any
>>specific instruction to load the data. The READ API implementation should
>>be concerned with efficient loading of the data, not the application. A
>>collection of objects should be a good enough indication to the READ API
>>implementation to prepare for loading this collection of objects.
>>
>>[BT] This is incorrect. **Of course!!!!* you load the data when you cross
>>a specific window!!! The application (in general) need not concern itself
>>but what's the harm in adding a "loaded" property ? Some applications may
>>wish to do that ! A collection is good enough and an app can not concern
>>itself .... and ignore this, some other may wish to do so. That's the
>>whole point. I think it's just you mis-interpet an intent to be
>>flexible and give **optimization** hints to actual load this is NOT the
>>case. This is just an API, the tool needs to have some smart developers.
FM: It seems to me that the load functions and properties should not be
part of the mandatory standard. They seem to be added as optional
functionality. They should be optional.
>>The proposal assumes a lack of knowledge of the current VPI standard
>>specification which is not appropriate nor necessary. For example, it
>>should not repeat the specification of the vpi_handle(), vpi_iterate()
>>nor explain the VPI naming conventions (section 29.1.2).
>>
>>[BT] Well that's the way the other chapters are: Assertion API/Coverage
>>API, why they are that way I don't know! Is this not an editorial change ???
FM: They should also be changed.
>>The proposal is written as a user manual and not as a standard
>>specification. The examples are useful but are not normative and should
>>be moved to an informative section. The examples make the specification
>>harder to comprehend. The term "user", which is used throughout the
>>document, is not accurate. Generally, the document uses this term to
>>refer to the application developer. However, there are other "users",
>>such as the API developer (which is using the specification while doing
>>his/her development) or the designer (which is using the tool) ...etc.
>>[BT] Yes your last comment indicates that "user" is more general :-)! It
>>can be a developer and or designer or anyone using it :-)! Anyway I think
>>Stu will boil all this down later ... just my 2 cents.
>>Collections should be a general concept. This concept is applicable in
>>other situations. As currently specified it is too specialized for the
>>collection of database objects.
>>
>>Some new concepts have been introduced but are incompletely defined such
>>as the time of a trvsObjCollection. We believe that this in particular
>>should not be defined.
FM: I think that there are things proposed in the specification which are
not vital to the standard, in particular defining the minimum time of a
trvscollection etc.. The standard should define the minimum set of API
functionality to define portability. This again seems like a vendor
convenience extension function, which can be found by querying the time of
each member of the collection. It should be removed from the standard.
>>[BT] explained above ..
>>
>>Nothing is specified on what happens if an object has been partially
>>dumped into the database.
>>
>>
>>
>>[BT] I thought I added this, I think it's in, this is old feedback.
FM: I don't think it is specified. For ex, if you only dumped a bit select
of a vector, what is happening if you obtain a handle to the vector and ask
for its trvsObj. What is the trvsObj going to return for the entire vector
or for the bits which were not probed? How are you going to find out which
bits were probed?
For a struct type object, do you get a trvsobj for the entire struct? or
one for each field? one for each scalar or vector?
>>Minor issues:
>>[BT] Thanks for all the edits, The editor (I think that's me) next week
>>must go through in detail ... except what is relevant to
>>philosophical/design issues of above ...
>>
>>Section 29.1.1:
>>The first diagram on page 1 shows a box containing "foreign". Since this
>>is not discussed at all in the document, this should be removed.
>>The introduction which talks about what systemVerilog adds in terms of
>>data types should be removed when final inclusion in the SV LRM.
>>If efficiency is not an API concern and is dealt behind the scenes, why
>>having the functions to indicate to load the data and query if the data
>>is loaded???
>>
>>[BT] Covered above...
>>Section 29.1.2: this section should be removed as it is common knowledge.
>>Section 29.2.1
>>The vpiObjCollection and vpiTrvsObjCollection should be removed.
>>The list of the object types which can be passed to vpi_get_value() is
>>inconsistent with the VPI information model defined for SV. Ex: Reg and
>>Reagarrays have disappeared.
>>We should not attempt to list the object types here.
>>[BT] Hmmm, this was my opinion all along (and the diagrams too!),
>>previous feedback suggested to do the list of things (which I might add
>>is not what other chapters e.g. assertion/coverage have done...).
FM: sorry if I had given you this feedback, I am not sure now what is the
best way to document this and be synchronized with the current VPI spec.
>>The vpiHasNoValue property is missing.
>>
>>
>>
>>Section 29.2.2.1 and 29.2.2.2
>>The separation between static and dynamic info has no precedent in VPI.
>>This classification has no advantage.
>>VpiIsLoaded should be removed.
>>
>>[BT] No precedent in Verilog yes, but in SV we did this in assertion
>>chapter !
>>Section 29.2.2.2
>>The reference to vpi_goto() should be removed.
>>vpi_get_time() can be used on a vpiTrvsObj, there is no need for the
>>additional function vpi_trvs_get_time().
>>
>>[BT] Nope vpi_get_time cannot be used without significant backward
>>compatability issues.
FM: You had claimed this before but I don't see any backward compatibility
issue. Please describe.
>>Section 29.2.3
>>This section is useless and should be removed.
>>
>>Section 29.3.2:
>>A single vpiType vpiCollection should be provided. No need to have
>>specialized classes.
>>
>>[BT] ....
>>Replace:
>> The first call gives NULL handles to the collection object and the
>> object to be added.
>>
>>with:
>> The first call provides a NULL handle to the collection object and the
>> object to be added to the collection.
>>
>>Another way to create a collection is to provide an iteration type method
>>and a reference handle.
>>For example, in order to create in one shot a collection of all the nets
>>which are declared in a scope:
>>colH = vpi_create(vpiCollectionK, 0, scopeH, vpiNet);
>>This is a very useful capability.
>>
>>Section 29.3.2.1 Operation on collections
>>vpi_read_load() should be removed.
>>
>>[BT] ....
>>vpi_goto() should be removed.
>>The behavior vpi_handle(vpiTrvsObjCollection, objCollectionH) is
>>unspecified with respect to unrecorded objects.
>>Do you get a trvsObj for a particular object if no data has been recorded
>>for that object? Do you get a NULL handle for that trvsObject? Do you
>>only get back a collection of non NULL trvsObj?
>>
>>[BT] ...
>>
>>Section 29.4:
>>The list of the traversable objects is incorrect.
>>
>>We should only have a vpiCollection type object. The fact is that there
>>is no specific additional properties about a vpiTrvsObjCollection of
>>vpiObjCollection; why having specialized classes?
>>The only possibility seems to be the vpiTrvsObjCollection 1-to-1 method
>>which has an incomplete specification.
>>
>>Section 29.6:
>>vpi_read_get_version() is not necessary VPI has already a function to
>>access version information:
>>vpi_get_vlog_info().
>>
>>vpi_load_extension() is not a READ API function, it is a function which
>>must be implemented by the application or tool which intends to call the
>>read api functions. This should be clearly distinguished.
>>
>>vpi_load_init(), vpi_load_init_create(), vpi_read_unload(),
>>vpi_read_load(), vpi_goto() should all be removed.
>>
>>vpi_goto() can easily be replaced by vpi_control() followed by
>>vpi_filter() with a criteria of vpiHasVC
>>
>>[BT] Yes, but what's the harm ?
FM: It is redundant and complicated the interface implementation.
>>Section 29.7:
>>I disagree with the reading of the data.
>>Accessing a vpiTrvsObj with vpi_handle() should be sufficient to access
>>the database data. There should not be any need to explicitly "load".
>>
>>[BT] There is no explicit load, this is just a hint ...
FM: Just accessing the handles should be the hint.
>>Section 29.7.1:
>>vpi_load_init() and vpi_load_init_create() are unnecessary
>>functions. Loading should be transparent to the application. The
>>indication of which data is to be loaded can be suggested by the handles
>>to the objects which have been obtained or gathered in a collection with
>>vpi_handle(), vpi_iterate(), or vpi_create().
>>
>>Sections 29.7.3, 29.7.3.1
>>These sections should be removed for reasons explained earlier.
>>
>>Section 29.7.4:
>>"So far we have outlined..."
>>should be removed.
>>
>>Why is the initial time of a trvsObj left to the tool implementation.
>>Shouldn't it start at the first time recorded in the database or at the
>>first time there is a value recorded for that object?
>>
>>[BT] No ... what if you are loading a "window-ed" in time ... I think it
>>is better to let apps be smart about this.
FM: ok
>>Section 29.7.2.3:
>>vpiHasNoValue property does not appear in the diagram featuring vpitrsObj.
>>
>>Section 29.7.5:
>>Some fragments of code which try to add a handle at a time to a
>>collection should be replaced with the capability of adding an entire
>>iteration at a time to the collection (see previous comments on vpi_create()).
>>
>>vpiReg is no longer a vpiType in SV VPI (at least in the latest spec sent
>>by Joao)
>>
>>[BT] ok ...
>>Section 29.7.7;
>>Remove any reference to vpi_goto().
>>
>>The time of a trvsObjCollection is undefined. Is it the smallest time
>>for a trvs object?
>>Some trvsObj of the collection may have different times. We should not
>>define the time of a trvsCollection. It seems that the code wants to
>>compare the time of each trvsobj with the time it passed to vpi_control()
>>in order to determine if a trvsObvj had a VC at that time. There is no
>>need to define the time of a trvsObjCollection.
>>
>>[BT] above ...
>>The vpi_goto() is unnecessary it can be replaced with the combination of
>>vpi_control() and vpi_filter() with a criteria of vpiHasVC.
>>
>>[BT] above ...
>>Section 29.8
>>Should be removed. No need to vpi_read_unload() : the tool itself should
>>implement its own optimizations for loading and unloading and not depend
>>on the application to instruct it to do this.
>>
>>[BT] ....
>>Section 29.9
>>The user_data field is unnecessary.
>>It is stated that the struct_version set to 1 for systemVerilog 3.1a. I
>>think that struct version value should not be specified by the standard.
>>It is tool implementation specific. What about if the tool wants to
>>provide other vpi function (vendor extensions), the tool will need to
>>generate a different version numbers for each s_vpi_extension structure
>>that is different. This is irrespective of what happens to the standard spec.
>>
>>[BT] Also agree... a lot of this is implementation dependent, yes! We
>>only need to focus on the read API portion ...
>>It is unclear who calls vpi_load_extension() and where the implementation
>>of this function resides.
>>
>>Section 29.10:
>>Section 29.10.1
>>vpi_control():
>>"Try to move" why try? Replace with "request to move"
>>remove reference to vpi_goto().
>>
>>[BT] "Try" is relatively speaking a construct in programming (as in
>>try/fail) ... request is not.
>>Section 29.10.2
>>Replace vpi_read_get_version() with vpi_get_vlog_info().
>>
>>Remove vpi_load_init() and vpi_load_init_create().
>>
>>Replace vpi_trvs_get_time() with vpi_get_time() and enhance
>>vpi_get_time() to handle vpiNextVc, vpiPrevVC etc...
>>
>>Remove vpi_read_load() and vpi_read_unload().
>>
>>vpi_create() is missing creating a collection with an iteration.
>>
>>Remove vpi_goto().
>>
>>
>>
>>[BT]...
>>
>>
>>
>>
>>
>>
>>
This archive was generated by hypermail 2b28 : Fri Jan 09 2004 - 21:19:14 PST