xls icon indicating copy to clipboard operation
xls copied to clipboard

Value from const array used in loop range fails assertion in the typechecker

Open koblonczek opened this issue 1 year ago • 1 comments

Describe the bug Typechecking fails on an internal assertion when a value from an array declared as const is used in a for loop range construct, e.g:

const FOO: u32[1] = [ u32:0 ];

#[test_proc]
proc ExampleProc {
    init {}
    config(_: chan<bool> out) {}

    next (_: ()) {
        for (i, ()): (u32, ()) in range(u32:0, u32:1) {
            let foobar = FOO[i];
            for (_, ()): (u32, ()) in range(u32:0, foobar) {}(());
        }(());
    }
}

Indexing array FOO with a constant doesn't trigger this bug, it must be indexed using a non-const, e.g. loop iterator to trigger it.

To Reproduce Steps to reproduce the behavior:

  1. Checkout this branch with minimal example in the forked repo
  2. Execute command bazel run //xls/modules/zstd:const_array_value_as_range
  3. Observe the following assertion stacktrace:
Error: INTERNAL: BytecodeEmitter could not find slot or binding for name: foobar @ xls/modules/zstd/const_array_value_as_range.x:10:17-10:23 stack: 0x55825becc82e: xls::dslx::BytecodeEmitter::HandleNameRefInternal()
0x55825becbc51: xls::dslx::BytecodeEmitter::HandleNameRef()
0x55825c2a46db: xls::dslx::NameRef::AcceptExpr()
0x55825beb8c90: xls::dslx::BytecodeEmitter::EmitExpression()
0x55825be39ead: xls::dslx::InterpretExpr()
0x55825be3aa4c: std::__1::__function::__func<>::operator()()
0x55825be4ce9d: std::__1::__function::__func<>::operator()()
0x55825be37373: xls::dslx::TypecheckParametricBuiltinInvocation()
0x55825be3309b: xls::dslx::TypecheckInvocation()
0x55825bde0708: std::__1::__function::__func<>::operator()()
0x55825be1d0a7: xls::dslx::DeduceInstantiation()
0x55825be1e083: xls::dslx::DeduceInvocation()
0x55825bdf743b: xls::dslx::(anonymous namespace)::DeduceVisitor::HandleInvocation()
0x55825c2a67cb: xls::dslx::Invocation::Accept()
0x55825bde8041: xls::dslx::Deduce()
0x55825bde0445: std::__1::__function::__func<>::operator()()
0x55825bee1dc8: xls::dslx::DeduceCtx::Deduce()
0x55825bee2bfc: xls::dslx::DeduceCtx::DeduceAndResolve()
0x55825be04f48: xls::dslx::(anonymous namespace)::DeduceLoopInitAndIterable()
0x55825bdf61dc: xls::dslx::(anonymous namespace)::DeduceVisitor::HandleFor()
0x55825c2a57ab: xls::dslx::For::Accept()
0x55825bde8041: xls::dslx::Deduce()
0x55825bde0445: std::__1::__function::__func<>::operator()()
0x55825bee1dc8: xls::dslx::DeduceCtx::Deduce()
0x55825bdedd74: xls::dslx::(anonymous namespace)::DeduceVisitor::HandleStatement()
0x55825c2a781b: xls::dslx::Statement::Accept()
0x55825bde8041: xls::dslx::Deduce()
0x55825bde0445: std::__1::__function::__func<>::operator()()
0x55825bee1dc8: xls::dslx::DeduceCtx::Deduce()
0x55825bdf23bb: xls::dslx::(anonymous namespace)::DeduceVisitor::HandleStatementBlock()
0x55825c2a70bb: xls::dslx::StatementBlock::Accept()
0x55825bde8041: xls::dslx::Deduce()
0x55825bde0445: std::__1::__function::__func<>::operator()()
0x55825bee1dc8: xls::dslx::DeduceCtx::Deduce()
0x55825bee2bfc: xls::dslx::DeduceCtx::DeduceAndResolve()
0x55825be062c0: xls::dslx::(anonymous namespace)::TypecheckLoopBody()
0x55825bdf620c: xls::dslx::(anonymous namespace)::DeduceVisitor::HandleFor()
0x55825c2a57ab: xls::dslx::For::Accept()
0x55825bde8041: xls::dslx::Deduce()
0x55825bde0445: std::__1::__function::__func<>::operator()()
0x55825bee1dc8: xls::dslx::DeduceCtx::Deduce()
0x55825bdedd74: xls::dslx::(anonymous namespace)::DeduceVisitor::HandleStatement()
0x55825c2a781b: xls::dslx::Statement::Accept()
0x55825bde8041: xls::dslx::Deduce()
0x55825bde0445: std::__1::__function::__func<>::operator()()
0x55825bee1dc8: xls::dslx::DeduceCtx::Deduce()
0x55825bdf23bb: xls::dslx::(anonymous namespace)::DeduceVisitor::HandleStatementBlock()
0x55825c2a70bb: xls::dslx::StatementBlock::Accept()
0x55825bde8041: xls::dslx::Deduce()
0x55825bde0445: std::__1::__function::__func<>::operator()()

Expected behavior Assertion in typechecker shouldn't fail and a more user-friendly message should be reported: one that mentions that foobar needs to be constexpr and perhaps suggests using unroll_for! for the outer loop instead.

Additional context This was discovered while working on DSLX tests for the ZSTD decoder in #1654. It's not a blocker as using unroll_for! correctly makes i constexpr (and by extension foobar as well) and hence solves the issue.

koblonczek avatar Oct 17 '24 13:10 koblonczek

Related: #1387

However, there, note that the repro produces a reasonable error for bit slices:

TypeInferenceError: Unable to resolve slice start to a compile-time constant.

It's array indexing that isn't handled as nicely.

mikex-oss avatar Oct 17 '24 16:10 mikex-oss