[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");
}else{
_parent = prechecked_cast<const MODEL_SUBCKT*>(model);
}
}else{
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
efficiency.
In this case, the code should be changed to:
/*--------------------------------------------------------------------------*/
void DEV_SUBCKT::expand()
{
BASE_SUBCKT::expand();
COMMON_SUBCKT* c = prechecked_cast<COMMON_SUBCKT*>(mutable_common());
assert(c);
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");
}else{
_parent = prechecked_cast<const MODEL_SUBCKT*>(model);
}
}else{
assert(find_looking_out(c->modelname()) == _parent);
}
assert(_parent->subckt());
assert(_parent->subckt()->params());
PARAM_LIST* pl = const_cast<PARAM_LIST*>(_parent->subckt()->params());
assert(pl);
c->_params.set_try_again(pl);
renew_subckt(_parent, this, scope(), &(c->_params));
subckt()->expand();
}
/*--------------------------------------------------------------------------*/
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.