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.

Sign up for GitHub

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

SystemVerilog logic array inside struct should warn on bad index #1296

Closed
veripoolbot opened this issue Mar 29, 2018 · 3 comments
Closed

SystemVerilog logic array inside struct should warn on bad index #1296

veripoolbot opened this issue Mar 29, 2018 · 3 comments
Labels
area: lint Issue involves SystemVerilog lint checking resolution: no fix needed Closed; no fix required (not a bug)

Comments

@veripoolbot
Copy link
Contributor

Author Name: Sergi Granell (@xerpi)
Original Redmine Issue: 1296 from https://www.veripool.org

It looks like when you have a logic array inside a struct, and you try to access a bit of that field directly, they indexing of the bits always start at 0 no matter the offset of the logic array.

module our();
	typedef struct packed {
		logic [31:20] imm;
		logic [19:15] rs1;
		logic [14:12] funct3;
		logic [11:7] rd;
		logic [6:0] opcode;
	} instruction_itype_t;

	/*
	 * The bit 31, same as instr.imm[11] will be 1.
	 */
	instruction_itype_t instr = 32'ha0008093; /* addi x1, x1, -1536 # aa00 */

	logic [11:0] imm;
	assign imm = instr.imm;

	initial begin
		$display("instr: 0x%X\n", instr);

		$display("instr.imm: %b (0x%X)", instr.imm, instr.imm);
		$display("imm:       %b (0x%X)\n", imm, imm);

		/*
		 * Should print 1 and not 0!!
		 */
		$display("instr.imm[11]: %d <-- should be 1 !!", instr.imm[11]);
		/*
		 * This prints 1 as expected.
		 */
		$display("imm[11]:       %d\n", imm[11]);

		/*
		 * Why is this working? That imm has 12 bits!
		 */
		$display("instr.imm[31]: %d", instr.imm[31]);
		$finish;
	end
endmodule
@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-03-30T00:35:25Z

No.

     logic [31:20] imm;
     $display("instr.imm[11]: %d <-- should be 1 !!", instr.imm[11]);

Imm is declared as [31:20] so reading bit 11 is illegal. Verilator decides that it will put a zero there to make faster code, but really it's an "X".

Indeed when I try this code on another simulator it gives an error message right on your "should be" extract.

Of course when you then assign it to a "logic [11:0]" you've reestablished the LSB/MSB range so the code is now ok, and as expected you get the right result.

So, Verilator is compliant run-time wise. However it really should give you a warning to point out the code is nonsensical, which is what I'll re-purpose this bug as a request to add.

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Sergi Granell (@xerpi)
Original Date: 2018-03-30T06:32:24Z

Wilson Snyder wrote:

No.

    logic [31:20] imm;
    $display("instr.imm[11]: %d <-- should be 1 !!", instr.imm[11]);

Imm is declared as [31:20] so reading bit 11 is illegal. Verilator decides that it will put a zero there to make faster code, but really it's an "X".

Indeed when I try this code on another simulator it gives an error message right on your "should be" extract.

Of course when you then assign it to a "logic [11:0]" you've reestablished the LSB/MSB range so the code is now ok, and as expected you get the right result.

So, Verilator is compliant run-time wise. However it really should give you a warning to point out the code is nonsensical, which is what I'll re-purpose this bug as a request to add.

Totally makes sense! Sorry for the confusion (I'm very new to Verilog) :)

@veripoolbot
Copy link
Contributor Author

Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2018-10-05T01:03:09Z

Wasn't a bug, forgot to close earlier.

@veripoolbot veripoolbot added area: lint Issue involves SystemVerilog lint checking resolution: nofixneeded resolution: no fix needed Closed; no fix required (not a bug) and removed resolution: nofixneeded labels Dec 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area: lint Issue involves SystemVerilog lint checking resolution: no fix needed Closed; no fix required (not a bug)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@veripoolbot