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

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

Open
koblonczek opened this issue Oct 17, 2024 · 1 comment
Labels
dslx DSLX (domain specific language) implementation / front-end enhancement New feature or request ux User experience (end-user invoking XLS and its related tools)

Comments

@koblonczek
Copy link

koblonczek commented Oct 17, 2024

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.

@mikex-oss
Copy link
Collaborator

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.

@proppy proppy added enhancement New feature or request dslx DSLX (domain specific language) implementation / front-end ux User experience (end-user invoking XLS and its related tools) labels Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dslx DSLX (domain specific language) implementation / front-end enhancement New feature or request ux User experience (end-user invoking XLS and its related tools)
Projects
Status: No status
Development

No branches or pull requests

3 participants