povik / yosys-slang

SystemVerilog frontend for Yosys
ISC License
55 stars 7 forks source link

Missynthesis with break statements #26

Closed povik closed 3 months ago

povik commented 3 months ago

The break statement support doesn't take the effects of the step expression into account. We missynthesize a priority encoder if written this way:

    always_comb begin: for_loop_priority_encoder
        integer i = 0;
        encoded = 1'b0;
        for (i = 0; i < INPUT_WIDTH; i++) begin
            if (bits[i]) begin
                break;
            end
        end
        encoded = i;
    end
alikates commented 3 months ago

The step expression is not evaluated inside the conditional branch:

            SwitchHelper b(current_case, vstate, {RTLIL::S0});
            b.sw->statement = &stmt;

            b.branch({substitute_rvalue(disable)}, [&]() {
                current_case->statement = &stmt.body;
                eval.push_frame(nullptr, disable);
                stmt.body.visit(*this);
                eval.pop_frame();
            });
            b.finish(netlist);

            for (auto step : stmt.steps)
                eval(*step);

            ncycles++;

I assume that the effect of the step should also be tracked inside the branch.

povik commented 3 months ago

Yes, but at the same time we need for it to be constant folded for the cv check, which may require some restructuring of the case tree to work out. (Specifically subsequent iterations need to be below earlier iterations on the case tree.) I can look into it later.

alikates commented 3 months ago

Okay, thanks!

povik commented 3 months ago

Should be addressed in e08d249