[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Gnucap-devel] another assertion 'bug'

From: al davis
Subject: Re: [Gnucap-devel] another assertion 'bug'
Date: Thu, 13 Dec 2012 06:43:15 -0500
User-agent: KMail/1.13.5 (Linux/2.6.32-5-amd64; KDE/4.4.5; x86_64; ; )

On Wednesday 12 December 2012, Felix Salfelder wrote:
> DEV_SUBCKT::expand() calls
> assert(model && model == _parent);
> if i read the code right, Savant instanciates "!_"
> subcircuits and then changes the dev_type to the 'device'
> attribute. i see nothing wrong with that....

(not reading Savant's code)

That is half correct.

It is correct, in parsing, to set_dev_type to the 'device' 
attribute, but incorrect to make them all subcircuits.

The correct procedure is to use the 'device' attribute to 
determine what kind of device to instanciate, not necessarily a 
subckt.  It might be, for example, a resistor.

The dev_type is not an arbitrary string.  What is valid there 
depends on what the base device is.  It's the first field in 
Verilog.  For those cases where it is a subcircuit (type X in 
Spice) the correct dev_type is the name associated with the 
subcircuit.  For those cases where it is a device like a diode 
that in Spice would have a .model card, it's the name associated 
with that .model card.

So, looking at that code in DEV_SUBCKT::expand()
  const CARD* model = find_looking_out(c->modelname());
  if (!_parent) {
    if(!dynamic_cast<const MODEL_SUBCKT*>(model)) {
      throw Exception_Type_Mismatch(long_label(), c->modelname(), "subckt");
      _parent = prechecked_cast<const MODEL_SUBCKT*>(model);
    assert(model && model == _parent);

The _parent is the .subckt in spice, or the module in verilog.

That block of code establishes an association between an 
instanciation and its parent.  Before this, all we know is that 
there is a parent subcircuit somewhere.  Expand finds it and 
makes a shallow copy of it.  That is .. it copies the device 
bodies, but may or may not copy the commons.  It may instead 
just link to them.

The true part  ..  if (!_parent)  ....  (read that "if no 
parent") ..  finds it and sets the link.  This is always the 
case the first time it is called.

The else part, which really does nothing, happens when expand is 
called again.  There is no need to repeat the lookup when it is 
already known, except to check it for validity.  If all is well, 
it will always be the same as it was the first time.

On looking at it today, I see that the code is wasteful in that 
it looks up again when it is not needed.  This is important 
because find_looking_out is slow.  That needs to be fixed, but 
it is not relevant to this.  All uses of find_looking_out, and 
find_looking_out itself need to be examined and reworked for 

In this case, the code should be changed to:
void DEV_SUBCKT::expand()
  COMMON_SUBCKT* c = prechecked_cast<COMMON_SUBCKT*>(mutable_common());
  if (!_parent) {
    const CARD* model = find_looking_out(c->modelname());
    if(!dynamic_cast<const MODEL_SUBCKT*>(model)) {
      throw Exception_Type_Mismatch(long_label(), c->modelname(), "subckt");
      _parent = prechecked_cast<const MODEL_SUBCKT*>(model);
    assert(find_looking_out(c->modelname()) == _parent);
  PARAM_LIST* pl = const_cast<PARAM_LIST*>(_parent->subckt()->params());

  renew_subckt(_parent, this, scope(), &(c->_params));

But this change is for efficiency, nothing to do with the 
problem at hand.

Back to the original question ...  If I understand correctly,
the problem is that he always starts with a subckt.  The correct
way is for find_type_in_string to find the device attribute,
which specifies the type.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]