RE: [sv-bc] Re: 1619 suggestions

Hi Cliff,

If I understand correctly, if an IP provider uses () in the default port
declaration, this means that the default port semantics WRT .* as
defined by 1619 apply to this port. In other words, if there is no
parent declaration for this signal, the instantiation will use the
default.  If we do not use the (), then the semantics change which force
the implicit .name semantics.  In other words if the parent does not
have the signal, then an error occurs.

So, if an IP provider were to start using default port values and wanted
to protect themselves from unconnected ports that have default values,
they would not use the () in order to force users to always have a
matching name in the parent and the child.

The distinction which becomes clear, then is whether or not the signal
exists in the parent and the child.  When an IP provider uses () there
does not have to be a match.  If they do not have the () then parent and
child MUST match, even when using a default.

Okay, that makes sense now. The bottom line is whether or not we want to
provide this enhancement.  This will complicate the proposal and I am
not convinced it provides a lot of benefit to it. I still go back to my
argument that IP providers need to be diligent and understand what they
are getting into.  If they really wanted to ensure that their IP not be
misused with default port connections then they shouldn't use them at
all.

But I don't see that this is something that can't be added in a later
version of the spec.  In fact, it shouldn't be too hard to add this
enhancement later such that it is backwards compatible with the existing
proposal.

-Tom 

>-----Original Message-----
>From: owner-sv-bc@server.eda.org [mailto:owner-sv-bc@server.eda.org] On
>Behalf Of Clifford E. Cummings
>Sent: Monday, December 03, 2007 4:07 PM
>To: Alsop, Thomas R
>Cc: sv-bc@server.eda.org
>Subject: [sv-bc] Re: 1619 suggestions
>
>Hi, Tom -
>
>Thanks for the reply. I hope you don't mind, I have copied the entire
>sv-bc because others might have the same questions.
>
>At 11:28 AM 12/3/2007, Alsop, Thomas R wrote:
>>Hi Cliff,  some follow up questions, see comments below. My
>>apologies if I am just missing your point.  Perhaps some follow up
>>mail can flush this all out.  Thanks for you inputs, they are
>>appreciated.  -Tom
>>
>>// Cliff's notes concerning 1619
>>//
>>// (1) The top example shows my concern over added default ports used
>with .*
>>// (2) The second example shows a proposed modification (albeit rather
>ugly)
>>// (3) The third example shows how to handle this with the current
>>syntax and no
>>// default ports
>>// (4) The forth example shows how to `include a core model in an
>>existing model
>>//--------------------------------------------------------------------
----
>---
>>
>>
>>//--------------------------------------------------------------------
----
>---
>>// IP Provider - New Model for accum module
>>//--------------------------------------------------------------------
----
>---
>>// IP Provider believes the better version of the model inverts the
>dataout
>>// MSB when not reset and has added an invert-MSB (inv_msb) control
input
>>// and set it to 1 so that future use of the model will invert the
dataout
>>// MSB by default
>>//--------------------------------------------------------------------
----
>---
>>module accum (
>>   output logic [7:0] dataout,
>>   input        [7:0] datain,
>>   input              clk, rst_n='1, inv_msb='1);
>>
>>   always @(posedge clk, negedge rst_n)
>>     if (!rst_n) dataout <= '0;
>>     else        dataout <= {inv_msb,7'b0}^datain; // New
functionality
>>endmodule
>>
>>I am not sure what you are implying by this code change.  I seems to
>>me that if an IP provider releases a new version of their code, it
>>must be backwards compatible, which implies that the inv_msb should
>>be set to '0.  They should understand the new implications of how
>>default ports work with .* if they are going to use this feature.
>>The enhancement to the code is then put into user documentation
>>telling them to set this port value to '1 if they want the new
>>functionality.  Any IP provider _must_ IMHO, be savvy enough to not
>>only ensure their code is backwards compatible but also document
>>these types of enhancements or users should find someone else.
>
>Fair point. Perhaps I am just too concerned about the knowledge of IP
>providers. We agree that an IP provider SHOULD release a version with
>new ports that would be backward compatible, but there are a number
>of Verilog book writers and IP providers that should just be sued for
>mal practice. If IP providers do the right thing, your proposal
>should be perfectly fine. Unfortunately, the proposal does not
>guarantee that IP providers will do the right thing, and my example
>above is a potential hazardous case.
>
>Like I tell people, Verilog is a language that allows you to hang
>yourself good if you don't know what you are doing while VHDL is a
>language that makes it hard to even build a scaffold.
>
>This is a potentially useful enhancement that again makes it easier
>to hang yourself if you don't know what you are doing.
>
>In recent years, I have been trying to tighten up the language with
>each new enhancement without going overboard.
>
>>  // All other models as defined in Mantis 1619
>>...
>>
>>module alu_accum4 (
>>    output [15:0] dataout,
>>    input   [7:0] ain, bin,
>>    input   [2:0] opcode,
>>    input         clk);
>>    wire    [7:0] alu_out;
>>
>>    alu   alu   (.*, .zero());
>>    // accum rst input is tied high AND inv_msb is now tied high
>>    // (inv_msb port did not exist before - added by IP provider)
>>    accum accum (.*, .dataout(dataout[7:0]), .datain(alu_out));
>>    xtend xtend (.*, .dout(dataout[15:8]),   .din(alu_out[7]));
>>endmodule
>>
>>//--------------------------------------------------------------------
----
>---
>>
>>// Alternative (Ugly) syntax - not much better or safer but does
require
>>// "responsible" action by the IP provider
>>module accum (
>>   output logic [7:0] dataout,
>>   input        [7:0] datain,
>>   input              clk,
>>   // Shows that this port can be omitted in .* connection
>>   input ()           rst_n='1,
>>   // missing () shows that this port must be listed by .*
>>   input              inv_msb='1);
>>
>>   always @(posedge clk, negedge rst_n)
>>     if (!rst_n) dataout <= '0;
>>     else        dataout <= {inv_msb,7'b0}^datain; // New
functionality
>>endmodule
>>
>>Wow, yeah, I think this is really stretching this new feature for a
>>case that should be understood well enough by IP providers in the
>>first place.  It adds complexity to an already complex feature and
>>is really only providing a safeguard for IP providers.  That said,
>>how is this going to guarantee that even this () gets used correctly
>>by the IP folks as opposed to just using the new feature as they
should?
>
>There are no guarantees that even this would fix the potential
>problems (which is why I am still leaning towards being the only
>no-vote). But with this type of syntax, an IP provider could not just
>say, "well, the user should always use a named port. My IP was never
>intended to be used without a named port." If an IP provider added ()
>to the new port, users could charge the IP provider with mal practice
>if they changed the default with the added pin. If the IP provider
>did not add () to the pin, the end user would get an error about the
>port without a corresponding net declaration, so the user gets
>immediate feedback that something has changed.
>
>>//--------------------------------------------------------------------
----
>---
>>
>>// Alternative instantiation without default ports
>>
>>module alu_accum4 (
>>    output [15:0] dataout,
>>    input   [7:0] ain, bin,
>>    input   [2:0] opcode,
>>    input         clk);
>>    wire    [7:0] alu_out;
>>
>>    alu   alu   (.*, .zero());
>>    accum accum (.*, .dataout(dataout[7:0]), .datain(alu_out),
.rst_n('1),
>>    `ifdef INV_MSB
>>    .inv_msb('1));
>>    `else
>>    .inv_msb('0));
>>    `endif
>>
>>    xtend xtend (.*, .dout(dataout[15:8]),   .din(alu_out[7]),
.rst('0));
>>endmodule
>>
>>I'm not sure what you are trying to show here.  I understand that
>>this is a way to set port values if you don't have default ports,
>>but the proposal is there so I don't have to do this.
>
>Understood and agreed. Although the new notation is convenient, the
>old notation was not terrible. That was the intent of this example.
>
>>//--------------------------------------------------------------------
----
>---
>>
>>// Alternative (currently legal) syntax for accum module
>>// use config instance statement to select the correct accum
>>// in the alu_accum4
>>
>>module accum (
>>   output logic [7:0] dataout,
>>   input        [7:0] datain,
>>   input              clk,
>>
>>   assign rst_n='1;
>>   assign inv_msb='1;
>>
>>But this doesn't even allow you to override these values upon
>>instantiation.  Sorry if I am just missing your point againJ  Please
>clarify.
>
>True. The point is, you can maintain multiple copies of wrappers
>around a common generic core, allow all of the wrappers to have the
>same module name and then use config "instance" statements to choose
>which configuration you would like for each instance.
>
>>   // module in module
>>   `include "accum_core.v"
>>endmodule
>>
>>// Separate file accum_core.v
>>module accum_core (
>>   output logic [7:0] dataout,
>>   input        [7:0] datain,
>>   input              clk,
>>   input              rst_n,
>>   input              inv_msb);
>>
>>   always @(posedge clk, negedge rst_n)
>>     if (!rst_n) dataout <= '0;
>>     else        dataout <= {inv_msb,7'b0}^datain; // New
functionality
>>endmodule
>>
>>//--------------------------------------------------------------------
----
>---
>
>My concern is that with SystemVerilog-2005 we added a very nice
>instantiation capability using .* that enforced a whole set of very
>useful design checks inside of a very concise and useful enhancement.
>I can see the value of default port values but I also see the danger
>and although some engineers would take advantage of the ability, I
>believe most will not. On the other hand, with this enhancement in
>place, ignorant IP providers could cause significant damage to the
>simple-case designs.
>
>I keep placing blame on IP providers but in fact, there is a whole
>group of self-taught SystemVerilog users that are going to abuse this
>capability as well.
>
>I am probably the only dissenting vote, so vote me down and there
>will be no hard feelings. I just hope the usage of this feature is
>rare enough that only the knowledgeable will use it.
>
>Regards - Cliff
>
>
>----------------------------------------------------
>Cliff Cummings - Sunburst Design, Inc.
>14314 SW Allen Blvd., PMB 501, Beaverton, OR 97005
>Phone: 503-641-8446 / FAX: 503-641-8486
>cliffc@sunburst-design.com / www.sunburst-design.com
>Expert Verilog, SystemVerilog, Synthesis and Verification Training
>
>
>--
>This message has been scanned for viruses and
>dangerous content by MailScanner, and is
>believed to be clean.

-- 
This message has been scanned for viruses and
dangerous content by MailScanner, and is
believed to be clean.