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

[ImportVerilog] Add case inside #7928

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ankolesn
Copy link

@ankolesn ankolesn commented Dec 1, 2024

Implementation of functionality for checking whether a value belongs to a set of expressions (like inside in SystemVerilog)

@ankolesn ankolesn changed the title Add case inside [ImportVerilog] Add case inside Dec 1, 2024
lib/Conversion/ImportVerilog/Statements.cpp Outdated Show resolved Hide resolved
test/Conversion/ImportVerilog/basic.sv Outdated Show resolved Hide resolved
lib/Conversion/ImportVerilog/Statements.cpp Outdated Show resolved Hide resolved
Comment on lines 2246 to 2251
always_comb begin
case (caseExpr)
32'h1, 32'h2, 32'h3: result = 1;
default: result = 0;
endcase
end
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your example is not a case inside statement. Maybe the example in the SystemVerilog IEEE Std § 12.5.4 is better. What do @ankolesn think 😃?

logic [2:0] status;
always @(posedge clock)
priority case (status) inside
1, 3 : task1; // matches 'b001 and 'b011
3'b0?0, [4:7]: task2;  // matches 'b000 'b010 'b0x0 'b0z0 'b100 'b101 'b110 'b111
endcase  // priority case fails all other values including 'b00x 'b01x 'bxxx

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your comments! I tried to fix it, you can check it 😃

@hailongSun2000 hailongSun2000 added Moore Arc Involving the `arc` dialect ImportVerilog and removed Arc Involving the `arc` dialect labels Dec 2, 2024
Comment on lines +2236 to +2302
module priority_case_test;
// CHECK: logic clock
logic clock;
// CHECK: logic [2:0] status
logic [2:0] status;

task task1;
// CHECK: task1 executed
$display("task1 executed at time %0t", $time);
endtask

task task2;
// CHECK: task2 executed
$display("task2 executed at time %0t", $time);
endtask

initial begin
// CHECK: clock initialized to 0
clock = 0;
// CHECK: clock toggles every 5 ns
forever #5 clock = ~clock;
end

initial begin
// CHECK: Test Case 1: status = 3'b001
status = 3'b001;
@(posedge clock);

// CHECK: Test Case 2: status = 3'b011
status = 3'b011;
@(posedge clock);

// CHECK: Test Case 3: status = 3'b010 (matches task2)
status = 3'b010;
@(posedge clock);

// CHECK: Test Case 4: status = 3'b100 (matches task2)
status = 3'b100;
@(posedge clock);

// CHECK: Test Case 5: status = 3'b00x (no match)
status = 3'b00x;
@(posedge clock);

// CHECK: Test Case 6: status = 3'b111 (matches task2)
status = 3'b111;
@(posedge clock);

// CHECK: Test Case 7: status = 3'bxxx (no match)
status = 3'bxxx;
@(posedge clock);

// CHECK: End simulation
$finish;
end

always @(posedge clock) begin
// CHECK: Entering priority case
priority case (status)
// CHECK: Matches 3'b001 or 3'b011, executes task1
3'b001, 3'b011: task1;
// CHECK: Matches 3'b0?0 or [3'b100:3'b111], executes task2
3'b0?0, [3'b100:3'b111]: task2;
// CHECK: No match, default case
default: $display("No match for status = %b at time %0t", status, $time);
endcase
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ankolesn, thanks a lot for working on this! One comment regarding your unit test: we don't simulate the design as part of the unit test, so things like $display(...) will not show up as prints in the output. Instead, the unit tests check that the expected IR operations get generated. For your PR, I'd add different examples that exercise interesting corner cases of your implementation. If you look through basic.sv, you should find existing tests for case statements that can give you some inspiration on how to do that. Then you can check if your code generates the correct IR ops for inside case statements.

case CaseStatementCondition::Inside:
mlir::emitError(loc, "unsupported set membership case statement");
return failure();
std::vector<Value> values;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SmallVector per MLIR coding guidelines.


if (values.size() == 1) {
cond = builder.create<moore::WildcardEqOp>(loc, caseExpr,
values.front());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: why use front here and [0] two lines later?

return failure();
}

if (values.size() == 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This conditional could be completely eliminated.

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

Successfully merging this pull request may close these issues.

4 participants