Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

import package is broken under multiply instantiated cells #542

Closed
veripoolbot opened this issue Aug 7, 2012 · 6 comments
Closed

import package is broken under multiply instantiated cells #542

veripoolbot opened this issue Aug 7, 2012 · 6 comments
Assignees
@wsnyder
Labels
resolution: fixed Closed; fixed

Comments

@veripoolbot
Copy link
Contributor

Author Name: Alex Solomatnikov
Original Redmine Issue: 542 from https://www.veripool.org
Original Date: 2012-08-07
Original Assignee: Wilson Snyder (@wsnyder)

This used to work 3 months ago:

package driver_definitions;

// Returns the maximum of two numbers
function automatic integer max;
	input integer a;
	input integer b;
	begin
		max = (a > b) ? a : b;
	end
endfunction

// Calculate the ceiling of log_2 of the input value
function automatic integer ceil_log2;
	input integer value;
	begin
		value = value - 1;
		for (ceil_log2 = 0; value > 0; ceil_log2 = ceil_log2 + 1)
			value = value >> 1;
	end
endfunction

endpackage

module rand_num_gen(
  ...
);

import driver_definitions::*;

...

localparam LFSR_DATA_WIDTH	= ceil_log2(LFSR_DATA_RANGE);
localparam LFSR_WIDTH		= max(4, ceil_log2(LFSR_DATA_RANGE + 1));

Now I get errors:

%Error: rand_num_gen.sv:46: Can't find definition of task/function: ceil_log2
%Error: rand_num_gen.sv:47: Can't find definition of task/function: max
@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2012-08-08T00:36:22Z

Sorry this works for me, please give a failing example in verilator test_regress format.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Alex Solomatnikov
Original Date: 2012-08-08T01:27:19Z

It looks like the reason for this issue is that verilator does not try to find/open a file with the name driver_definitions.sv, as it used to do (or as VCS does). So, I get an error when I try to compile just this module:

verilato -sv --cc rand_num_gen.sv --top-module rand_num_gen -Wno-fatal -I.
%Error: rand_num_gen.sv:30: syntax error, unexpected IDENTIFIER, expecting PACKAGE-IDENTIFIER or STRING
%Error: Exiting due to 1 error(s)

If I add:

`include "driver_definitions.sv"

then it works.

Similarly, test_regress/t/t_package_abs.v has package definition in the same file, not in a separate file as it is usually done.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2012-08-08T01:49:34Z

No, verilator has never implemented automatic package resolution, and it fails on both revisions the same.

Also neither does VCS. At present VCS requires all packages to be listed on the command line, or equivalently in a file included off from the command line. Even `including a package from a module that is found by automatic library lookup is NOT sufficient. About 3 months ago I had a long debate with them on getting them to fix this mess but they stonewalled. Yes, it should be obvious that this is useful....

Anyhow if you can isolate the test case that passes on the old rev and fails now I'll fix it; there is a lot changed there and I have little doubt that something was broken.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Alex Solomatnikov
Original Date: 2012-08-08T18:52:35Z

You are right - it is not an include issue.

Here is reduced test case:

package defs;

function automatic integer max;
         input integer a;
         input integer b;
         begin
                 max = (a > b) ? a : b;
         end
endfunction

function automatic integer log2;
         input integer value;
         begin
                 value = value >> 1;
                 for (log2 = 0; value > 0; log2 = log2 + 1)
                         value = value >> 1;
         end
endfunction

function automatic integer ceil_log2;
         input integer value;
         begin
                 value = value - 1;
                 for (ceil_log2 = 0; value > 0; ceil_log2 = ceil_log2 + 1)
                         value = value >> 1;
         end
endfunction

endpackage

module gen(
         clk,
         reset_n,
         enable,
         ready,
         rand_num,
         is_less_than
);

import defs::*;

parameter RAND_NUM_WIDTH        = "";
parameter RAND_NUM_MIN          = "";
parameter RAND_NUM_MAX          = "";

localparam DATA_RANGE           = RAND_NUM_MAX - RAND_NUM_MIN + 1;
localparam DATA_WIDTH           = ceil_log2(DATA_RANGE);
localparam WIDTH                = max(4, ceil_log2(DATA_RANGE + 1));

input                           clk;
input                           reset_n;
input                           enable;
output                          ready;
output  [RAND_NUM_WIDTH-1:0]    rand_num;
output                          is_less_than;

endmodule

module test(
         clk,
         reset_n,
         enable,
         ready,
         count
);

import defs::*;

parameter COUNT_WIDTH           = "";

parameter POWER_OF_TWO          = "";

parameter MIN_COUNT             = "";
parameter MAX_COUNT             = "";

localparam MIN_EXPONENT         = ceil_log2(MIN_COUNT);
localparam MAX_EXPONENT         = log2(MAX_COUNT);
localparam EXPONENT_WIDTH       = ceil_log2(MAX_EXPONENT + 1);

input                           clk;
input                           reset_n;
input                           enable;
output                          ready;
output  [COUNT_WIDTH-1:0]  count;

generate
if (POWER_OF_TWO == 1)
begin : power_of_two_true
         wire    [EXPONENT_WIDTH-1:0]    rand_exponent_out;

         gen rand_exponent (
                 .clk            (clk),
                 .reset_n        (reset_n),
                 .enable         (enable),
                 .ready          (ready),
                 .rand_num       (rand_exponent_out));
         defparam rand_exponent.RAND_NUM_WIDTH   = EXPONENT_WIDTH;
         defparam rand_exponent.RAND_NUM_MIN             = MIN_EXPONENT;
         defparam rand_exponent.RAND_NUM_MAX             = MAX_EXPONENT;

         assign count = 1 << rand_exponent_out;
end
else
begin : power_of_two_false
         gen rand_count (
                 .clk            (clk),
                 .reset_n        (reset_n),
                 .enable         (enable),
                 .ready          (ready),
                 .rand_num       (count));
         defparam rand_count.RAND_NUM_WIDTH = COUNT_WIDTH;
         defparam rand_count.RAND_NUM_MIN   = MIN_COUNT;
         defparam rand_count.RAND_NUM_MAX   = MAX_COUNT;
end
endgenerate

endmodule

Latest verilator version:

~/verilator/verilator_bin -sv --cc -Wno-fatal --error-limit 1000 test.v --top-module test
%Warning-PINMISSING: test.v:91: Cell has missing pin: is_less_than
%Warning-PINMISSING: Use "/* verilator lint_off PINMISSING */" and lint_on around source to disable this message.
%Warning-PINMISSING: test.v:105: Cell has missing pin: is_less_than
%Error: test.v:47: Can't find definition of task/function: ceil_log2
%Error: test.v:48: Can't find definition of task/function: max
%Error: Exiting due to 2 error(s)

Older version:

/tools/verilator/verilator-3.833/bin/verilator_bin -sv --cc -Wno-fatal --error-limit 1000 test.v --top-module test
%Warning-PINMISSING: test.v:91: Cell has missing pin: is_less_than
%Warning-PINMISSING: Use "/* verilator lint_off PINMISSING */" and lint_on around source to disable this message.
%Warning-PINMISSING: test.v:105: Cell has missing pin: is_less_than
%Warning-WIDTH: test.v:76: Operator FUNCREF 'ceil_log2' expects 32 bits on the Function Argument, but Function Argument's VARREF 'MIN_COUNT' generates 1 bits.
%Warning-WIDTH: test.v:77: Operator FUNCREF 'log2' expects 32 bits on the Function Argument, but Function Argument's VARREF 'MAX_COUNT' generates 1 bits.
%Warning-LITENDIAN: test.v:54: Little bit endian vector: MSB < LSB of bit range: -1:0
%Warning-LITENDIAN: test.v:84: Little bit endian vector: MSB < LSB of bit range: -1:0

Note that 3.833 includes tri-state fixes.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2012-08-09T02:00:19Z

Fixed in git towards 3.841.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2012-09-04T00:14:42Z

In 3.841.

@veripoolbot veripoolbot added the resolution: fixed Closed; fixed label Dec 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@wsnyder wsnyder

Labels
resolution: fixed Closed; fixed
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@wsnyder @veripoolbot