Skip to content
New issue

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

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

Already on GitHub? # to your account

Automatic Function Produces Construct with Infinite Loop in Yosys #275

Open
wrs225 opened this issue Jan 23, 2024 · 6 comments
Open

Automatic Function Produces Construct with Infinite Loop in Yosys #275

wrs225 opened this issue Jan 23, 2024 · 6 comments

Comments

@wrs225
Copy link

wrs225 commented Jan 23, 2024

I have the following design:

module fp_addsub (
    input logic [31:0] a,
    input logic [31:0] b,
    input logic subtract,
    output logic [31:0] y
);
    logic [31:0] b_neg;
    logic a_sign, b_sign;
    logic [7:0] a_exp, b_exp;
    logic [23:0] a_frac, b_frac;  // Increase fraction size to 24 bits
    logic y_sign;
    logic [7:0] y_exp;
    logic [23:0] y_frac;  // Increase fraction size to 24 bits
    logic [47:0] y_frac_intermediate;  // Intermediate fraction for normalization extended to 48 bits
    logic [7:0] y_exp_intermediate;  // Intermediate exponent for normalization
    logic [5:0] shift_amount;  // Shift amount from priority encoder

    // Negate b if subtract is high
    assign b_neg = subtract ? {b[31]^1'b1, b[30:0]} : b;

    // Priority encoder to find the leading one
    function automatic [5:0] priority_encoder;
        input [47:0] in;
        integer i;
        priority_encoder = 6'b000000; // Default value
        for (i = 47; i >= 0; i = i - 1) begin
            if (in[i]) begin
                priority_encoder = 23 - i;
                i = -1; // Break the loop by setting i to an invalid value
            end
        end
    endfunction

    // Add or subtract
    always_comb begin
        // Extract fields
        a_sign     = a[31];
        b_sign     = b_neg[31];
        a_exp = a[30:23];
        b_exp = b_neg[30:23];
        a_frac = {1'b1, a[22:0]};  // Add implicit leading 1
        b_frac = {1'b1, b_neg[22:0]};  // Add implicit leading 1

        // Perform addition or subtraction
        if (a_exp > b_exp) begin
            y_exp_intermediate = a_exp;
            if (b_sign ^ a_sign) begin
                y_frac_intermediate = a_frac - (b_frac >> (a_exp - b_exp));  // Corrected shift amount
            end else begin
                y_frac_intermediate = a_frac + (b_frac >> (a_exp - b_exp));  // Corrected shift amount
            end
            y_sign = a_sign;
        end else if (a_exp < b_exp) begin
            y_exp_intermediate = b_exp;
            if (b_sign ^ a_sign) begin
                y_frac_intermediate = b_frac - (a_frac >> (b_exp - a_exp));  // Corrected shift amount
            end else begin
                y_frac_intermediate = (a_frac >> (b_exp - a_exp)) + b_frac;  // Corrected shift amount
            end
            y_sign = b_sign;
        end else begin
            y_exp_intermediate = a_exp;
            if (a_frac >= b_frac) begin
                if (b_sign ^ a_sign) begin
                    y_frac_intermediate = a_frac - b_frac;  // No shift when exponents are equal
                end else begin
                    y_frac_intermediate = a_frac + b_frac;  // No shift when exponents are equal
                end
                y_sign = a_sign;
            end else begin
                if (b_sign ^ a_sign) begin
                    y_frac_intermediate = b_frac - a_frac;  // No shift when exponents are equal
                end else begin
                    y_frac_intermediate = b_frac + a_frac;  // No shift when exponents are equal
                end
                y_sign = b_sign;
            end
        end

        // Normalize result using priority encoder
        if (a == 32'b0 && b == 32'b0) begin
            shift_amount = 6'b000000;  // No shift if both inputs are zero
        end else begin
            shift_amount = y_frac_intermediate[47] ? 6'b000000 : priority_encoder(y_frac_intermediate);
        end

        // Shift fraction and adjust exponent
        if (shift_amount[5]) begin  // If the most significant bit is 1, shift_amount is negative
            y_frac = y_frac_intermediate >> -shift_amount;  // Right shift
        end else begin
            y_frac = y_frac_intermediate << shift_amount;  // Left shift
        end
        y_exp = y_exp_intermediate - {{2{shift_amount[5]}}, shift_amount};  // Sign extend shift_amount and subtract from y_exp_intermediate

    end

    // Check for infinity and pack fields into output
    always_comb begin
        // Check if either input is infinity
        if ((a[30:23] == 8'hFF && a[22:0] == 23'b0) && (b_neg[30:23] == 8'hFF && b_neg[22:0] == 23'b0)) begin
            // If both inputs are infinity and the operation is subtraction, the output is NaN
            if (subtract && (a_sign ^ b_sign)) begin
                y = {1'b0, 8'hFF, 23'h400000};
            end else if (a_sign ^ b_sign) begin
                y = {a_sign, 8'hFF, 23'b0};
            end else begin
                y = {y_sign, y_exp, y_frac[22:0]};  // Only take the lower 23 bits of the fraction
            end
        end else if ((a[30:23] == 8'hFF && a[22:0] == 23'b0) || (b_neg[30:23] == 8'hFF && b_neg[22:0] == 23'b0)) begin
            // If either input is infinity (but not both), set the output to infinity as well
            y = {a_sign | b_sign, 8'hFF, 23'b0};
        end else if (y_frac_intermediate == 48'b0) begin
            y = 32'b0;
        end else begin
            // If neither input is infinity and the output is not zero, perform regular assignment
            y = {y_sign, y_exp, y_frac[22:0]};  // Only take the lower 23 bits of the fraction
        end
    end
endmodule

When pushing the design through sv2v, the priority encoder gets converted into this:

module fp_addsub (
	a,
	b,
	subtract,
	y
);
	input wire [31:0] a;
	input wire [31:0] b;
	input wire subtract;
	output reg [31:0] y;
	wire [31:0] b_neg;
	reg a_sign;
	reg b_sign;
	reg [7:0] a_exp;
	reg [7:0] b_exp;
	reg [23:0] a_frac;
	reg [23:0] b_frac;
	reg y_sign;
	reg [7:0] y_exp;
	reg [23:0] y_frac;
	reg [47:0] y_frac_intermediate;
	reg [7:0] y_exp_intermediate;
	reg [5:0] shift_amount;
	assign b_neg = (subtract ? {b[31] ^ 1'b1, b[30:0]} : b);
	function automatic [5:0] priority_encoder;
		input [47:0] in;
		integer i;
		reg [0:1] _sv2v_jump;
		begin
			_sv2v_jump = 2'b00;
			begin : sv2v_autoblock_1
				integer _sv2v_value_on_break;
				for (i = 47; i >= 0; i = i - 1)
					if (_sv2v_jump < 2'b10) begin
						_sv2v_jump = 2'b00;
						if (in[i]) begin
							priority_encoder = 23 - i;
							_sv2v_jump = 2'b11;
						end
						_sv2v_value_on_break = i;
					end
				if (!(_sv2v_jump < 2'b10))
					i = _sv2v_value_on_break;
				if (_sv2v_jump != 2'b11)
					_sv2v_jump = 2'b00;
			end
			if (_sv2v_jump == 2'b00) begin
				priority_encoder = 6'b000000;
				_sv2v_jump = 2'b11;
			end
		end
	endfunction
	always @(*) begin
		a_sign = a[31];
		b_sign = b_neg[31];
		a_exp = a[30:23];
		b_exp = b_neg[30:23];
		a_frac = {1'b1, a[22:0]};
		b_frac = {1'b1, b_neg[22:0]};
		if (a_exp > b_exp) begin
			y_exp_intermediate = a_exp;
			if (b_sign ^ a_sign)
				y_frac_intermediate = a_frac - (b_frac >> (a_exp - b_exp));
			else
				y_frac_intermediate = a_frac + (b_frac >> (a_exp - b_exp));
			y_sign = a_sign;
		end
		else if (a_exp < b_exp) begin
			y_exp_intermediate = b_exp;
			if (b_sign ^ a_sign)
				y_frac_intermediate = b_frac - (a_frac >> (b_exp - a_exp));
			else
				y_frac_intermediate = (a_frac >> (b_exp - a_exp)) + b_frac;
			y_sign = b_sign;
		end
		else begin
			y_exp_intermediate = a_exp;
			if (a_frac >= b_frac) begin
				if (b_sign ^ a_sign)
					y_frac_intermediate = a_frac - b_frac;
				else
					y_frac_intermediate = a_frac + b_frac;
				y_sign = a_sign;
			end
			else begin
				if (b_sign ^ a_sign)
					y_frac_intermediate = b_frac - a_frac;
				else
					y_frac_intermediate = b_frac + a_frac;
				y_sign = b_sign;
			end
		end
		if ((a == 32'b00000000000000000000000000000000) && (b == 32'b00000000000000000000000000000000))
			shift_amount = 6'b000000;
		else
			shift_amount = (y_frac_intermediate[47] ? 6'b000000 : priority_encoder(y_frac_intermediate));
		if (shift_amount[5])
			y_frac = y_frac_intermediate >> -shift_amount;
		else
			y_frac = y_frac_intermediate << shift_amount;
		y_exp = y_exp_intermediate - {{2 {shift_amount[5]}}, shift_amount};
	end
	always @(*)
		if (((a[30:23] == 8'hff) && (a[22:0] == 23'b00000000000000000000000)) && ((b_neg[30:23] == 8'hff) && (b_neg[22:0] == 23'b00000000000000000000000))) begin
			if (subtract && (a_sign ^ b_sign))
				y = 32'h7fc00000;
			else if (a_sign ^ b_sign)
				y = {a_sign, 31'h7f800000};
			else
				y = {y_sign, y_exp, y_frac[22:0]};
		end
		else if (((a[30:23] == 8'hff) && (a[22:0] == 23'b00000000000000000000000)) || ((b_neg[30:23] == 8'hff) && (b_neg[22:0] == 23'b00000000000000000000000)))
			y = {a_sign | b_sign, 31'h7f800000};
		else if (y_frac_intermediate == 48'b000000000000000000000000000000000000000000000000)
			y = 32'b00000000000000000000000000000000;
		else
			y = {y_sign, y_exp, y_frac[22:0]};
endmodule

When isolated this causes yosys to hang forever.

function automatic [5:0] priority_encoder;
		input [47:0] in;
		integer i;
		reg [0:1] _sv2v_jump;
		begin
			_sv2v_jump = 2'b00;
			begin : sv2v_autoblock_1
				integer _sv2v_value_on_break;
				for (i = 47; i >= 0; i = i - 1)
					if (_sv2v_jump < 2'b10) begin
						_sv2v_jump = 2'b00;
						if (in[i]) begin
							priority_encoder = 23 - i;
							_sv2v_jump = 2'b11;
						end
						_sv2v_value_on_break = i;
					end
				if (!(_sv2v_jump < 2'b10))
					i = _sv2v_value_on_break;
				if (_sv2v_jump != 2'b11)
					_sv2v_jump = 2'b00;
			end
			if (_sv2v_jump == 2'b00) begin
				priority_encoder = 6'b000000;
				_sv2v_jump = 2'b11;
			end
		end
	endfunction

When replacing this portion with the following:

	function automatic [5:0] priority_encoder;
		input [47:0] in;
		integer i;
		begin
			priority_encoder = 6'b000000;
			for (i = 47; i >= 0; i = i - 1)
				if (in[i]) begin
					priority_encoder = 23 - i;
					i = -1;
				end
		end
	endfunction

It both becomes valid Verilog and stops Yosys from hanging. My version of sv2v is sv2v v0.0.11-15-gdeed2d9

@zachjs
Copy link
Owner

zachjs commented Jan 29, 2024

I presume i = -1; is supposed to be break; in your original example, otherwise there would be no _sv2v_jump in the output.

Can you share an end-to-end example for what is causing Yosys to hang? I wasn't immediately able to reproduce it with:
sv2v foo.sv > foo.v && yosys -Qp "read_verilog foo.v; hierarchy; proc; opt -full"

@zachjs
Copy link
Owner

zachjs commented Jan 29, 2024

What version of Yosys are you using?

@wrs225
Copy link
Author

wrs225 commented Jan 30, 2024

My apologies, my original file was slightly wrong. I pulled it from a GitHub repo.

module fp_addsub (
    input logic [31:0] a,
    input logic [31:0] b,
    input logic subtract,
    output logic [31:0] y
);

    logic [31:0] b_neg;
    logic a_sign, b_sign;
    logic [7:0] a_exp, b_exp;
    logic [23:0] a_frac, b_frac;  // Increase fraction size to 24 bits
    logic y_sign;
    logic [7:0] y_exp;
    logic [23:0] y_frac;  // Increase fraction size to 24 bits
    logic [47:0] y_frac_intermediate;  // Intermediate fraction for normalization extended to 48 bits
    logic [7:0] y_exp_intermediate;  // Intermediate exponent for normalization
    logic [5:0] shift_amount;  // Shift amount from priority encoder

    // Negate b if subtract is high
    assign b_neg = subtract ? {b[31]^1'b1, b[30:0]} : b;

    // Priority encoder to find the leading one
    function automatic [5:0] priority_encoder (input [47:0] in);
        integer i;
        for (i=47; i>=0; i=i-1) begin
            if (in[i]) begin
                return 23 - i;
            end
        end
        return 6'b000000;  // Return 0 if no one is found
    endfunction

    // Add or subtract
    always_comb begin
        // Extract fields
        a_sign     = a[31];
        b_sign     = b_neg[31];
        a_exp = a[30:23];
        b_exp = b_neg[30:23];
        a_frac = {1'b1, a[22:0]};  // Add implicit leading 1
        b_frac = {1'b1, b_neg[22:0]};  // Add implicit leading 1

        // Perform addition or subtraction
        if (a_exp > b_exp) begin
            y_exp_intermediate = a_exp;
            if (b_sign ^ a_sign) begin
                y_frac_intermediate = a_frac - (b_frac >> (a_exp - b_exp));  // Corrected shift amount
            end else begin
                y_frac_intermediate = a_frac + (b_frac >> (a_exp - b_exp));  // Corrected shift amount
            end
            y_sign = a_sign;
        end else if (a_exp < b_exp) begin
            y_exp_intermediate = b_exp;
            if (b_sign ^ a_sign) begin
                y_frac_intermediate = b_frac - (a_frac >> (b_exp - a_exp));  // Corrected shift amount
            end else begin
                y_frac_intermediate = (a_frac >> (b_exp - a_exp)) + b_frac;  // Corrected shift amount
            end
            y_sign = b_sign;
        end else begin
            y_exp_intermediate = a_exp;
            if (a_frac >= b_frac) begin
                if (b_sign ^ a_sign) begin
                    y_frac_intermediate = a_frac - b_frac;  // No shift when exponents are equal
                end else begin
                    y_frac_intermediate = a_frac + b_frac;  // No shift when exponents are equal
                end
                y_sign = a_sign;
            end else begin
                if (b_sign ^ a_sign) begin
                    y_frac_intermediate = b_frac - a_frac;  // No shift when exponents are equal
                end else begin
                    y_frac_intermediate = b_frac + a_frac;  // No shift when exponents are equal
                end
                y_sign = b_sign;
            end
        end

        // Normalize result using priority encoder
        if (a == 32'b0 && b == 32'b0) begin
            shift_amount = 6'b000000;  // No shift if both inputs are zero
        end else begin
            shift_amount = y_frac_intermediate[47] ? 6'b000000 : priority_encoder(y_frac_intermediate);
        end

        // Shift fraction and adjust exponent
        if (shift_amount[5]) begin  // If the most significant bit is 1, shift_amount is negative
            y_frac = y_frac_intermediate >> -shift_amount;  // Right shift
        end else begin
            y_frac = y_frac_intermediate << shift_amount;  // Left shift
        end
        y_exp = y_exp_intermediate - {{2{shift_amount[5]}}, shift_amount};  // Sign extend shift_amount and subtract from y_exp_intermediate

    end

    // Check for infinity and pack fields into output
    always_comb begin
        // Check if either input is infinity
        if ((a[30:23] == 8'hFF && a[22:0] == 23'b0) && (b_neg[30:23] == 8'hFF && b_neg[22:0] == 23'b0)) begin
            // If both inputs are infinity and the operation is subtraction, the output is NaN
            if (subtract && (a_sign ^ b_sign)) begin
                y = {1'b0, 8'hFF, 23'h400000};
            end else if (a_sign ^ b_sign) begin
                y = {a_sign, 8'hFF, 23'b0};
            end else begin
                y = {y_sign, y_exp, y_frac[22:0]};  // Only take the lower 23 bits of the fraction
            end
        end else if ((a[30:23] == 8'hFF && a[22:0] == 23'b0) || (b_neg[30:23] == 8'hFF && b_neg[22:0] == 23'b0)) begin
            // If either input is infinity (but not both), set the output to infinity as well
            y = {a_sign | b_sign, 8'hFF, 23'b0};
        end else if (y_frac_intermediate == 48'b0) begin
            y = 32'b0;
        end else begin
            // If neither input is infinity and the output is not zero, perform regular assignment
            y = {y_sign, y_exp, y_frac[22:0]};  // Only take the lower 23 bits of the fraction
        end
    end
endmodule

With this file, I can reproduce with this:

sv2v foo.sv > foo.v && yosys -Qp "read_verilog foo.v; synth -top fp_addsub"

My Yosys version is:

Yosys 0.34+14 (git sha1 11b9deba9, clang 10.0.0-4ubuntu1 -fPIC -Os)

@wrs225
Copy link
Author

wrs225 commented Mar 13, 2024

Were you able to reproduce the issue?

@ldoolitt
Copy link

ldoolitt commented Nov 7, 2024

Just for fun, I tried this with sv2v v0.0.12-19-g7808819.
Yes, yosys 0.46 has problems dealing with the resulting verilog. The chatter ends with:

2.13. Executing SHARE pass (SAT-based resource sharing).
Found 4 cells in module fp_addsub that may be considered for resource sharing.
  Analyzing resource sharing options for $shr$foo.v:74$22 ($shr):
Killed

.. with a long time delay before the Killed.

Also, verilator gives a %Warning-LITENDIAN for reg [0:1] _sv2v_jump; Fixing that one sounds easy and innocuous.

@zachjs
Copy link
Owner

zachjs commented Nov 7, 2024

Does the converted output actually contain an infinite loop? As far as I can tell, Yosys is able to unroll the loop just fine (and indeed the jump conversion goes out of its way to make it so). I can imagine why this might trip some passes up, but I wonder if share is really behaving as intended here. Are commercial tools able to synthesize sv2v's output without issue?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants