Randomization and race conditions

Hello, I’m trying to learn SystemVerilog, and I’m currently doing randomization.

I have following package and a class inside it:

package RGB_specific;
		
		parameter true = 1'b1;
    parameter false = 1'b0;
    typedef struct {rand byte R,G,B;} RGB_t;

    class RGB_random;
        rand RGB_t pixel;
        constraint Reds   {pixel.R > pixel.B; pixel.R > pixel.G;}  //make pixel mostly red
        constraint Blues  {pixel.B > pixel.R; pixel.B > pixel.G;}  //mostly blue
        constraint Greens {pixel.G > pixel.R; pixel.G > pixel.B;}  //mostly green
        function void SetReds;
            Reds.constraint_mode(true);
            Greens.constraint_mode(false);
            Blues.constraint_mode(false);
        endfunction
        function void SetGreens;
            Reds.constraint_mode(false);
            Greens.constraint_mode(true);
            Blues.constraint_mode(false);
        endfunction
        function void SetBlues;
            Reds.constraint_mode(false);
            Greens.constraint_mode(false);
            Blues.constraint_mode(true);
        endfunction
        function new;  //constructor
            SetReds;
        endfunction
    endclass

endpackage

Now, I have my DUT that does nothing except pass its inputs to outputs (it’s clocked)

module DUT(clock,px_in,px_out);

    import RGB_specific::*;
    input logic clock;
    input RGB_random px_in;
    output RGB_random px_out;
   
    // DUT just checks that data arrives
		always @(posedge clock) begin 
		 $display("px_in: %h %h %h at time %t",
		        px_in.pixel.R, px_in.pixel.G, px_in.pixel.B, $time);
		    px_out <= px_in;
		end
  
endmodule : DUT

And I tried to connect this in my TOP module:

module top;

    import RGB_specific::*;
   
    bit clock;
    
    //clock generator
    always begin
    	#5
    	clock = 0;
    	#5
    	clock = 1; 	
    end
   
    RGB_random px_in, px_out;
    	
    //construct objects
    initial begin
    		px_in = new;
    		px_out = new;
    end
   
   //instantiate dut
    DUT U1(clock,px_in,px_out);
    
    //always randomize values of input
    always @(posedge clock) begin
    	void'(px_in.randomize());
    end
	
		//when any of the output values change, print new output values
		always @(px_out.pixel.R or px_out.pixel.G or px_out.pixel.B) begin
			$display("px_out: %h %h %h at time %t", px_out.pixel.R,px_out.pixel.G,px_out.pixel.B, $time);
		end
   
endmodule : top

Q1: Now, this all seemingly works fine, outputs get values from inputs. But, I have a question, as you see I also watch $time, and here is the output my QuestaSim gives me:

px_in: 00 00 00 at time 10

px_out: 7b 02 28 at time 10

px_in: 7b 02 28 at time 20

px_out: d2 c7 8a at time 20

px_in: d2 c7 8a at time 30

First it shows me outputs, and then tells me inputs with #10 delay. Everything is clocked on posedge, so I don’t see how this can happen.

Q2: But, even worse, I tried to avoid race condition and randomize on negative edge

always @(negedge clock) begin
    	void'(px_in.randomize());
    end

And then, I don’t get any of the output lines… Like its combinational, not sequential circuit.

Q3: QuestaSim also gives me this warning:
Variable ‘/top/px_out’, driven via a port connection, is multiply driven. See top.sv(24). (it points to a line where constructor is called). Why is that?

Thank you in advance!

In reply to mirkic:

You need to keep track of the movement of object handles, versus the data you are assigning inside those objects.

In the top level, you construct two objects, placing their handles in px_in and px_out. Yet px_out is connected to the output of the DUT, which makes a continuous assignment to it. It’s illegal to have both procedural and continuous assignments to the same variable. Inside the DUT, you copy the handle, not the object from px_in to px_out. Now you are left with only one object.

I think what you want is to put the construction of objects inside the always block.

 always @(posedge clock) begin
        px_in = new();
    	if (!px_in.randomize()) $error("randomization failed");
    end

In reply to dave_59:

Hi Dave, thank you for your reply! I have tried to run your solution but it produced Fatal error:

(SIGSEGV) Bad handle or reference.

Since then, I have tried to “upgrade” my code to use interfaces…

Package is the same, and I have this interface

interface pix(input bit clk);
	import RGB_specific::*;
	
	RGB_random px_in, px_out;
	
	modport dut(input clk, input px_in, output px_out);
	modport tb(input clk, input px_out, output px_in);
	
endinterface
module DUT(input bit clock, pix.dut pif);

    import RGB_specific::*;
       
    // DUT just checks that data arrives
		always_ff @(posedge clock) begin 
		 $display("px_in: %h %h %h at time %t",
		        pif.px_in.pixel.R, pif.px_in.pixel.G, pif.px_in.pixel.B, $time);
		    //pif.px_out <= pif.px_in;
		    //now copies each of 3 components separetely
		    pif.px_out.pixel.R <= pif.px_in.pixel.R;
		    pif.px_out.pixel.G <= pif.px_in.pixel.G;
		    pif.px_out.pixel.B <= pif.px_in.pixel.B;
		end
  
endmodule : DUT
module testb(input bit clk, pix.tb pif);

    import RGB_specific::*;
         
    //always randomize values of input
    always_ff @(negedge clk) begin
    	if (!pif.px_in.randomize()) $error("randomization failed");
    	
    	$display(pif.px_in.pixel.R,,pif.px_in.pixel.G,,pif.px_in.pixel.B,,$time);
    end
	
		//when any of the output values change, print new output values
		always @(pif.px_out.pixel.R or pif.px_out.pixel.G or pif.px_out.pixel.B) begin
			$display("px_out: %h %h %h at time %t", pif.px_out.pixel.R, pif.px_out.pixel.G, pif.px_out.pixel.B, $time);
		end
   
endmodule : testb
module top;
	bit clk;
	
	always begin
		#5 clk = ~clk;
	end
	
	pix interf_pix(clk);
	
	//initialization of objects of RGB_random class within this interface
	initial begin
		interf_pix.px_in = new;
		interf_pix.px_out = new;
	end
	
	DUT U1(clk,interf_pix);
	testb TST(clk, interf_pix); 
	
	initial begin
		#50
		$display("setgreen");
		interf_pix.px_in.SetGreens;
	end

endmodule

I think this now works as it should.

As I understand creating objects of classes, you need to allocate memory for that objects and thats why you call a constructor (“new”). So, thats why I tried to allocate both objects. And then, I guess only change values in that single object.

Do you mind explaining where my logic is flawed please?

Thanks in advance.

In reply to mirkic:
Your logic is flawed because you still do not understand the difference between the object as a transaction versus the values inside the object. The reason for your fatal error is because you are trying to find the value of an class member before you have a valid handle to the object. And if you are trying to send random pixels, what happens if you have two of the same sets of pixel values in two or more consecutive pixels?