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

Non-constant case item not supported #227

Open
zyedidia opened this issue Jun 16, 2021 · 1 comment
Open

Non-constant case item not supported #227

zyedidia opened this issue Jun 16, 2021 · 1 comment
Labels
A-codegen Area: Code generation. C-bug Category: This is a bug. E-help-wanted Call for Participation: Help wanted on this issue. L-vlog Language: Verilog and SystemVerilog.

Comments

@zyedidia
Copy link

It seems like moore requires that case items evaluate to integer constants even though this is more restrictive than the SystemVerilog standard. For example, this code should be valid SystemVerilog:

module test
    (
        input logic current_state,
        output logic [2:0] get_ready, get_set, get_going
    );

    always_comb begin
        {get_ready, get_set, get_going} = 3'b000;
        unique case (1'b1)
            current_state[0]: get_ready = '1;
            current_state[1]: get_set   = '1;
            current_state[2]: get_going = '1;
        endcase
    end
endmodule

But moore reports an error:

error: value is not constant
  --> test.sv:16:13-26:
   | 
   |             current_state[0]: get_ready = '1;
   |             ^^^^^^^^^^^^^                    

thread 'main' panicked at 'case constant evaluates to non-integer', /home/zyedidia/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/src/libstd/macros.rs:13:23
@fabianschuiki
Copy link
Owner

Thanks @zyedidia for reporting this issue. This is indeed something that should work, and off the top of my head I don't think there is anything technical that prevents having non-constants in this case statement.

The relevant code is here:

moore/src/svlog/codegen.rs

Lines 2278 to 2301 in eb4d44c

// Determine the constant value of the label.
let way_const = self.constant_value_of(way_expr, env);
let (_, special_bits, x_bits) = match &way_const.kind {
ValueKind::Int(v, s, x) => (v, s, x),
_ => panic!("case constant evaluates to non-integer"),
};
let way_expr = self.emit_const(way_const, env, self.span(way_expr))?;
let way_width = self.llhd_type(way_expr).unwrap_int();
// Generate the comparison mask based on the case kind.
let mask = match kind {
ast::CaseKind::Normal => None,
ast::CaseKind::DontCareZ => {
let mut mask = special_bits.clone();
mask.difference(x_bits);
mask.negate();
Some(mask)
}
ast::CaseKind::DontCareXZ => {
let mut mask = special_bits.clone();
mask.negate();
Some(mask)
}
};

It looks like the only thing that requires a bit of care are casex/casez, where the code generation currently wants a constant value to check for X and Z bits. This is due to the current lack of support for four-/nine-valued logic in LLHD (it's planned, but not implemented yet) -- so the codegen tries to do this ahead of time for the common case of casex/casez.

The above code could instead first check if the value is a constant (there's a compiler query for that), and if yes perform the whole masking/matching magic, and otherwise just use the non-constant value for the comparison.

@fabianschuiki fabianschuiki added A-codegen Area: Code generation. C-bug Category: This is a bug. E-help-wanted Call for Participation: Help wanted on this issue. L-vlog Language: Verilog and SystemVerilog. labels Jun 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation. C-bug Category: This is a bug. E-help-wanted Call for Participation: Help wanted on this issue. L-vlog Language: Verilog and SystemVerilog.
Projects
None yet
Development

No branches or pull requests

2 participants