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

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

Comments

@veripoolbot
Copy link

@veripoolbot veripoolbot commented Mar 29, 2018


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

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Mar 30, 2018


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

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Mar 30, 2018


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

This comment has been minimized.

Copy link
Author

@veripoolbot veripoolbot commented Oct 5, 2018


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


Wasn't a bug, forgot to close earlier.

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

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.