Force constant evaluation in slang front-end

[dinoruic]

Attempt to fix Issue #9489 which noted a difference in constant evaluation when building with slang at head.

Following the suggestion made by @fabianschuiki in #9489, I replaced expr.selector().getConstant() by context.evaluateConstant(expr.selector()) which will force constant evaluation.

However, it is unclear to me how to fix the tests defined in test/Conversion/ImportVerilog/basic.sv for both slang at version 9.1 (which is the default in CIRCT) and for slang at head.

Let me be a bit more specific. Here's the piece of SystemVerilog in question:

module PortsTop; // line 1703
...
// lines starting at 1741
  wire x3, y3;
  wire [2:0] z3;
  wire [1:0] w3;
  // CHECK: [[X3:%.+]] = moore.read %x3
  // CHECK: [[Y3:%.+]] = moore.read %y3
  // CHECK: [[V2:%.+]] = moore.extract_ref %z3 from 0
  // CHECK: [[V1:%.+]] = moore.extract_ref %z3 from 1
  // CHECK: [[V0:%.+]] = moore.extract_ref %z3 from 2
  // CHECK: [[V0_READ:%.+]] = moore.read [[V0]]
  // CHECK: [[C1:%.+]] = moore.extract_ref %w3 from 0
  // CHECK: [[C0:%.+]] = moore.extract_ref %w3 from 1
  // CHECK: [[C0_READ:%.+]] = moore.read [[C0]]
  // CHECK: [[V1_VALUE:%.+]], [[C1_VALUE:%.+]] = moore.instance "p3" @MultiPorts(
  // CHECK-SAME:   a0: [[X3]]: !moore.l1
  // CHECK-SAME:   a1: [[Y3]]: !moore.l1
  // CHECK-SAME:   v0: [[V0_READ]]: !moore.l1
  // CHECK-SAME:   v2: [[V2]]: !moore.ref<l1>
  // CHECK-SAME:   c0: [[C0_READ]]: !moore.l1
  // CHECK-SAME: ) -> (
  // CHECK-SAME:   v1: !moore.l1
  // CHECK-SAME:   c1: !moore.l1
  // CHECK-SAME: )
  // CHECK-NEXT: moore.assign [[V1]], [[V1_VALUE]]
  // CHECK-NEXT: moore.assign [[C1]], [[C1_VALUE]]
  MultiPorts p3(x3, y3, z3, w3);
...

When running the import test with CIRCT with slang v9.1, we get the currently expected result (only printing the relevant output lines):

$ circt-translate --import-verilog test/Conversion/ImportVerilog/basic.sv
....
// lines 1435 and following:
    %8 = moore.read %x3 : <l1>
    %9 = moore.read %y3 : <l1>
    %10 = moore.extract_ref %z3 from 0 : <l3> -> <l1>
    %11 = moore.extract_ref %z3 from 1 : <l3> -> <l1>
    %12 = moore.extract_ref %z3 from 2 : <l3> -> <l1>
    %13 = moore.read %12 : <l1>
    %14 = moore.extract_ref %w3 from 0 : <l2> -> <l1>
    %15 = moore.extract_ref %w3 from 1 : <l2> -> <l1>
    %16 = moore.read %15 : <l1>
....

If we use slang at head, we get the following output which does not pass the FileCheck test on basic.sv:

$ circt-translate --import-verilog test/Conversion/ImportVerilog/basic.sv
....
// lines 1435 and following:
    %8 = moore.read %x3 : <l1>
    %9 = moore.read %y3 : <l1>
    %10 = moore.extract_ref %z3 from 0 : <l3> -> <l1>
    %11 = moore.extract_ref %z3 from 1 : <l3> -> <l1>
    %12 = moore.read %z3 : <l3>
    %13 = moore.constant 2 : i32
    %14 = moore.dyn_extract %12 from %13 : l3, i32 -> l1
    %15 = moore.extract_ref %w3 from 0 : <l2> -> <l1>
    %16 = moore.read %w3 : <l2>
    %17 = moore.constant 1 : i32
    %18 = moore.dyn_extract %16 from %17 : l2, i32 -> l1

....

Note how the moor.extract_ref %z3 turns into something with moore.dyn_extract. According to @fabianschuiki in #9489, this is a consequence of slang not evaluating constants unless they are needed to determine types.

This PR contains the suggested fix changing expr.selector().getConstant() to context.evaluateConstant(expr.selector()). So here is the output with these changes with slang at v9.1 showing that the fix does not impact the test:

$ circt-translate --import-verilog test/Conversion/ImportVerilog/basic.sv
...
// lines 1435 and following:
    %8 = moore.read %x3 : <l1>
    %9 = moore.read %y3 : <l1>
    %10 = moore.extract_ref %z3 from 0 : <l3> -> <l1>
    %11 = moore.extract_ref %z3 from 1 : <l3> -> <l1>
    %12 = moore.extract_ref %z3 from 2 : <l3> -> <l1>
    %13 = moore.read %12 : <l1>
    %14 = moore.extract_ref %w3 from 0 : <l2> -> <l1>
    %15 = moore.extract_ref %w3 from 1 : <l2> -> <l1>
    %16 = moore.read %15 : <l1>

But with slang at head, we get:

$ circt-translate --import-verilog test/Conversion/ImportVerilog/basic.sv
...
// lines 1435 and following:
    %8 = moore.read %x3 : <l1>
    %9 = moore.read %y3 : <l1>
    %10 = moore.extract_ref %z3 from 0 : <l3> -> <l1>
    %11 = moore.extract_ref %z3 from 1 : <l3> -> <l1>
    %12 = moore.read %z3 : <l3>
    %13 = moore.extract %12 from 2 : l3 -> l1
    %14 = moore.extract_ref %w3 from 0 : <l2> -> <l1>
    %15 = moore.read %w3 : <l2>
    %16 = moore.extract %15 from 1 : l2 -> l1

So applying the fix will avoid the moore.dyn_extract, however, the output is different for slang at v9.1 and slang at master. Ideally CIRCT with slang at v9.1 and slang at head would output the same, so I wouldn't have to temporarily patch expected test output on my end.

Do you see a way to fix the ImportVerilog code such that the same code gets emitted in both cases?

If not, I can still merge this PR if there's interest in changing the constant evaluation behavior.

[fabianschuiki]

Thanks a lot @dinoruic for taking the time and looking into a fix for this! 🥳 It looks like with Slang 9.1, we do the extraction on the z3 ref and then read the ref:

    %12 = moore.extract_ref %z3 from 2 : <l3> -> <l1>
    %13 = moore.read %12 : <l1>

But with Slang head we do the read of the z3 ref first and then do the extraction:

    %12 = moore.read %z3 : <l3>
    %13 = moore.extract %12 from 2 : l3 -> l1

I'm not quite sure why this happens. Have you tried extracting the SV source lines that correspond to this into a small test case to reproduce what you are seeing here? It's strange that the other extractions remain extract_ref.