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

This code stalls instead of erroring #1131

Open
parker-research opened this issue May 21, 2024 · 2 comments
Open

This code stalls instead of erroring #1131

parker-research opened this issue May 21, 2024 · 2 comments

Comments

@parker-research
Copy link

parker-research commented May 21, 2024

The following code contains a bug, where signals reference each other in a sort of non-deterministic infinite loop. Currently, IVerilog stalls during the vvp execution stage. I believe that it should instead provide an error during compile, or end in an error during the exeuction.

IVerilog version: OSSCAD Release 2024-05-01

Execution commands:

iverilog -g2012 -Wall -o ./comp.vvp ./attempted_solution_code.sv ./testbench_code.sv
vvp ./comp.vvp

Input File 1: attempted_solution_code.sv (generated by a large language model):

`timescale 1 ps/1 ps

module top_module(
    input mode,
    input too_cold,
    input too_hot,
    input fan_on,
    output heater,
    output aircon,
    output fan
);

    assign heater = (mode && too_cold) || (fan_on && !aircon);
    assign aircon = (!mode && too_hot) || (fan_on && !heater);
    assign fan = (heater || aircon) || fan_on;

endmodule

Input File 2: testbench_code.sv:

`timescale 1 ps/1 ps
`define OK 12
`define INCORRECT 13
module reference_module(
	input mode,
	input too_cold, 
	input too_hot,
	input fan_on,
	output heater,
	output aircon,
	output fan
);
	
	assign fan = (mode ? too_cold : too_hot) | fan_on;
	assign heater = (mode & too_cold);
	assign aircon = (~mode & too_hot);
	
endmodule


module stimulus_gen (
	input clk,
	output reg too_cold, too_hot, mode, fan_on,
	output reg[511:0] wavedrom_title,
	output reg wavedrom_enable
);


// Add two ports to module stimulus_gen:
//    output [511:0] wavedrom_title
//    output reg wavedrom_enable

	task wavedrom_start(input[511:0] title = "");
	endtask
	
	task wavedrom_stop;
		#1;
	endtask	



	initial begin
		{too_cold, too_hot, mode, fan_on} <= 4'b0010;
		@(negedge clk);
		wavedrom_start("Winter");
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b0010;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b0010;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b1010;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b1011;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b0010;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b0011;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b0010;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b0110;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b1110;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b0111;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b1111;
		@(negedge clk) wavedrom_stop();

		{too_cold, too_hot, mode, fan_on} <= 4'b0000;
		@(negedge clk);
		wavedrom_start("Summer");
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b0000;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b0000;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b0100;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b0101;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b0000;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b0001;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b0000;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b1000;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b1100;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b1001;
			@(posedge clk) {too_cold, too_hot, mode, fan_on} <= 4'b1101;
		@(negedge clk) wavedrom_stop();
		
		repeat(200)
			@(posedge clk, negedge clk) {too_cold, too_hot, mode, fan_on} <= $random;
		
		#1 $finish;
	end
	
endmodule

module tb();

	typedef struct packed {
		int errors;
		int errortime;
		int errors_heater;
		int errortime_heater;
		int errors_aircon;
		int errortime_aircon;
		int errors_fan;
		int errortime_fan;

		int clocks;
	} stats;
	
	stats stats1;
	
	
	wire[511:0] wavedrom_title;
	wire wavedrom_enable;
	int wavedrom_hide_after_time;
	
	reg clk=0;
	initial forever
		#5 clk = ~clk;

	logic mode;
	logic too_cold;
	logic too_hot;
	logic fan_on;
	logic heater_ref;
	logic heater_dut;
	logic aircon_ref;
	logic aircon_dut;
	logic fan_ref;
	logic fan_dut;


	wire tb_match;		// Verification
	wire tb_mismatch = ~tb_match;
	
	


	initial begin 
		$dumpfile("./new_wave.vcd");
		$dumpvars(1, stim1.clk, tb_mismatch ,mode,too_cold,too_hot,fan_on,heater_ref,heater_dut,aircon_ref,aircon_dut,fan_ref,fan_dut );
	end


	stimulus_gen stim1 (
		.clk,
		.* ,
		.mode,
		.too_cold,
		.too_hot,
		.fan_on );
	reference_module good1 (
		.mode,
		.too_cold,
		.too_hot,
		.fan_on,
		.heater(heater_ref),
		.aircon(aircon_ref),
		.fan(fan_ref) );
		
	top_module top_module1 (
		.mode,
		.too_cold,
		.too_hot,
		.fan_on,
		.heater(heater_dut),
		.aircon(aircon_dut),
		.fan(fan_dut) );

	
	bit strobe = 0;
	task wait_for_end_of_timestep;
		repeat(5) begin
			strobe <= !strobe;  // Try to delay until the very end of the time step.
			@(strobe);
		end
	endtask	

	
	final begin
		if (stats1.errors_heater) $display("Hint: Output '%s' has %0d mismatches. First mismatch occurred at time %0d.", "heater", stats1.errors_heater, stats1.errortime_heater);
		else $display("Hint: Output '%s' has no mismatches.", "heater");
		if (stats1.errors_aircon) $display("Hint: Output '%s' has %0d mismatches. First mismatch occurred at time %0d.", "aircon", stats1.errors_aircon, stats1.errortime_aircon);
		else $display("Hint: Output '%s' has no mismatches.", "aircon");
		if (stats1.errors_fan) $display("Hint: Output '%s' has %0d mismatches. First mismatch occurred at time %0d.", "fan", stats1.errors_fan, stats1.errortime_fan);
		else $display("Hint: Output '%s' has no mismatches.", "fan");

		$display("Hint: Total mismatched samples is %1d out of %1d samples\n", stats1.errors, stats1.clocks);
		$display("Simulation finished at %0d ps", $time);
		$display("Mismatches: %1d in %1d samples", stats1.errors, stats1.clocks);
	end
	
	// Verification: XORs on the right makes any X in good_vector match anything, but X in dut_vector will only match X.
	assign tb_match = ( { heater_ref, aircon_ref, fan_ref } === ( { heater_ref, aircon_ref, fan_ref } ^ { heater_dut, aircon_dut, fan_dut } ^ { heater_ref, aircon_ref, fan_ref } ) );
	// Use explicit sensitivity list here. @(*) causes NetProc::nex_input() to be called when trying to compute
	// the sensitivity list of the @(strobe) process, which isn't implemented.
	always @(posedge clk, negedge clk) begin

		stats1.clocks++;
		if (!tb_match) begin
			if (stats1.errors == 0) stats1.errortime = $time;
			stats1.errors++;
		end
		if (heater_ref !== ( heater_ref ^ heater_dut ^ heater_ref ))
		begin if (stats1.errors_heater == 0) stats1.errortime_heater = $time;
			stats1.errors_heater = stats1.errors_heater+1'b1; end
		if (aircon_ref !== ( aircon_ref ^ aircon_dut ^ aircon_ref ))
		begin if (stats1.errors_aircon == 0) stats1.errortime_aircon = $time;
			stats1.errors_aircon = stats1.errors_aircon+1'b1; end
		if (fan_ref !== ( fan_ref ^ fan_dut ^ fan_ref ))
		begin if (stats1.errors_fan == 0) stats1.errortime_fan = $time;
			stats1.errors_fan = stats1.errors_fan+1'b1; end

	end
endmodule

@parker-research parker-research changed the title This code runs very slowly, no indication why This code stalls instead of erroring May 21, 2024
@caryr
Copy link
Collaborator

caryr commented May 21, 2024

Icarus does check for and report some possible infinite loops, but it does not currently search for combinational infinite loops. We have added instrumentation in the code the compiler generates that can help debug these kind of issues when it is enabled, but I can't remember if continuous assignments are instrumented. I think it is currently just procedural code so you would need to change from an assign to always_comb to find the actual loop. The other possible way to track this down is to figure out which transition is triggering the combinational loop and then think about how the variables are changing and how it could create a combinational oscillation.

I've been looking at LLM generated code for a bit and for some things the generated code is okay, but there are often cases where the code is incorrect from an implementation perspective or just plain invalid Verilog. If you tell the LLM it has generated invalid code it will come back and say yes you are correct, but that seems more of a socially acceptable response when the validity of the generated code is challenged.

@parker-research
Copy link
Author

Is this code even valid Verilog? I assumed it wasn't (as it's outside of an always_comb, which could be used to make a ring oscillator with this sort of code intentionally), but I could be wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants