[dslx] ir-interpreter fails with internal error resolving parametric constants in proc
- Dominant language
- C++
- Stars
- 1.9k
- Forks
- 283
- Avg merge
- 2d 10h
- Merged PRs (30d)
- 135
Description
**Describe the bug**
(note, this requires but #4909 to be fixed first to even be visible. I've applied my proposed patch there to then be able to get further in my xls code, then ran into this).
Consider the following code
```Rust
proc ParametricProc {
c: chan out,
}
impl ParametricProc {
fn new(c: chan out) -> Self {
ParametricProc { c }
}
fn next(self) {
let tok = join();
send(tok, self.c, N);
}
}
#[test]
proc ParametricProcTest {
done: chan out,
}
impl ParametricProcTest {
fn new(done: chan out) -> Self {
let (s, r) = chan("c");
let dut = ParametricProc<3>::new(s);
dut.spawn();
ParametricProcTest { done }
}
fn next(self) {
send(join(), self.done, true);
}
}
```
This has an issue when converting to IR
```
bazel-bin/xls/dslx/interpreter_main reduced_parametric_proc.x --evaluator=ir-interpreter
=== Source Location Trace: ===
xls/dslx/ir_convert/function_converter.cc:580
xls/dslx/ir_convert/function_converter.cc:3152
xls/dslx/ir_convert/function_converter.cc:678
xls/dslx/ir_convert/function_converter.cc:5009
xls/dslx/ir_convert/function_converter.cc:4981
xls/dslx/ir_convert/function_converter.cc:4294
xls/dslx/ir_convert/ir_converter.cc:365
xls/dslx/ir_convert/ir_converter.cc:486
xls/dslx/run_routines/ir_test_runner.cc:237
xls/dslx/run_routines/run_routines.cc:1031
xls/dslx/interpreter_main.cc:280
```
(this works fine with `--evaluator=dslx-interpreter`)
**Expected behavior**
No crash :)
**Environment (this can be helpful for troubleshooting):**
Compiled from head, with patches suggested in #4909 appliied.
**Additional context**
_(AI dislaimer: the following is antigravity-automated root caused, just edited for brevity, but not manually verified for soundness)_
### Root cause
The root cause of the error is that proc-level parametric bindings are currently not evaluated or registered into node_to_ir_ during proc IR conversion:
1. Missing Parametric Environment Initialization: In `ConvertProcDef` (function_converter.cc:4256), `SetParametricEnv(&env)` is never called, so `parametric_env_map_` is not populated for the proc conversion context.
2. Unimplemented Parametric Binding Handling: In `InitProcDefBuilder` (function_converter.cc:3786), line 3821 contains an explicit TODO _Deal with the parametric binding here..._
Unlike old-style procs (`HandleProcNextFunction`), `InitProcDefBuilder` does not iterate through `proc_def->parametric_bindings()` to evaluate each parametric parameter and populate `node_to_ir_` using `DefConst & DefAlias`.
3. Failure during `NameRef` Lookup: When `ConvertProcDef` subsequently converts `next()` (function_converter.cc:4294) expressions referencing parametrics trigger `HandleNameRef` (function_converter.cc:907).
### To resolve this issue
* In `ConvertProcDef` (function_converter.cc:4259), invoke `SetParametricEnv(&env);`
* In `InitProcDefBuilder` (function_converter.cc:3786), iterate over `proc_def->parametric_bindings(),` evaluate each binding value from `parametric_env_map_`, call `DefConst(parametric_binding, param_value)`, and alias via `DefAlias(parametric_binding, parametric_binding->name_def())`
### Proposed patch
_NB: AI generated_
```patch
--- a/xls/dslx/ir_convert/function_converter.cc
+++ b/xls/dslx/ir_convert/function_converter.cc
@@ -3786,6 +3786,7 @@ absl::Status FunctionConverter::DefineProcDefChannelOrArrayIfLocal(
absl::Status FunctionConverter::InitProcDefBuilder(const ProcDef* proc_def,
const ParametricEnv& env) {
+ SetParametricEnv(&env);
absl::btree_set parametric_keys;
// Include parametric values in the mangled names of non-top procs only.
@@ -3821,8 +3822,39 @@ absl::Status FunctionConverter::InitProcDefBuilder(const ProcDef* proc_def,
tokens_.push_back(implicit_token);
- // TODO: https://github.com/google/xls/issues/4125 - Deal with the parametric
- // bindings here, using `HandleProcNextFunction` as a rough guide.
+ for (ParametricBinding* parametric_binding : proc_def->parametric_bindings()) {
+ if (parametric_binding->type_annotation()
+ ->IsAnnotation()) {
+ continue;
+ }
+
+ VLOG(5) << "Resolving parametric binding: "
+ << parametric_binding->ToString();
+
+ std::optional parametric_value =
+ GetParametricBinding(parametric_binding->identifier());
+ XLS_RET_CHECK(parametric_value.has_value());
+ XLS_ASSIGN_OR_RETURN(std::unique_ptr parametric_type,
+ ResolveType(parametric_binding->name_def()));
+ XLS_RET_CHECK(!parametric_type->IsMeta());
+
+ XLS_ASSIGN_OR_RETURN(TypeDim parametric_width_ctd,
+ parametric_type->GetTotalBitCount());
+ XLS_ASSIGN_OR_RETURN(int64_t bit_count, parametric_width_ctd.GetAsInt64());
+ Value param_value;
+ if (parametric_value->IsSigned()) {
+ XLS_ASSIGN_OR_RETURN(int64_t bit_value,
+ parametric_value->GetBitValueViaSign());
+ param_value = Value(SBits(bit_value, bit_count));
+ } else {
+ XLS_ASSIGN_OR_RETURN(uint64_t bit_value,
+ parametric_value->GetBitValueViaSign());
+ param_value = Value(UBits(bit_value, bit_count));
+ }
+ DefConst(parametric_binding, param_value);
+ XLS_RETURN_IF_ERROR(
+ DefAlias(parametric_binding, /*to=*/parametric_binding->name_def()));
+ }
VLOG(3) << "Proc has " << constant_deps_.size() << " constant deps";
```
Contributor guide
Research direction
Start with the reduced proc example and the ir-interpreter command in the issue, then inspect InitProcDefBuilder and ConvertProcDef in xls/dslx/ir_convert/function_converter.cc. Trace how proc-level parametric bindings reach NameRef lookup, using HandleProcNextFunction as the cited comparison. Done means the example converts and runs without the internal error under --evaluator=ir-interpreter.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- compilers
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 55/100