google / google/xls

[DSLX] IR lowering fails for impl-style procs due to unhandled ProcType (self) parameter

Open
#4,926 0 comments 0 reactions 1 assignee Claimed by @richmckeever View on GitHub
Dominant language
C++
Stars
1.9k
Forks
283
Avg merge
2d 10h
Merged PRs (30d)
135

Description

**Describe the bug**

Consider the following code:

```Rust
fn my_func(x: s32) -> s32 {
if x < s32:0 { -x } else { x }
}

#[test]
proc MinimalProcTest {
done: chan out,
}

impl MinimalProcTest {
fn new(done: chan out) -> Self {
MinimalProcTest { done }
}

fn next(self) {
let x = my_func(s32:-42);
send(join(), self.done, x == s32:42);
}
}
```

(After some baseline fix applied patch from #4925 )
We get a crash:

```
$ bazel-bin/xls/dslx/interpreter_main test_minimal.x --evaluator=ir-interpreter
Error: UNIMPLEMENTED: IR lowering for impl-style procs is not yet supported: MinimalProcTest { done: chan(uN[1], dir=out) }
=== Source Location Trace: ===
xls/dslx/ir_convert/ir_conversion_utils.cc:100
xls/dslx/ir_convert/ir_conversion_utils.cc:149
xls/dslx/ir_convert/ir_conversion_utils.cc:132
xls/dslx/ir_convert/ir_conversion_utils.cc:149
xls/dslx/ir_convert/function_converter.cc:864
xls/dslx/ir_convert/function_converter.cc:3545
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
```

**Expected behavior**
no crashh.

**Environment (this can be helpful for troubleshooting):**
Compiled from head, patch from #4925 applied.

**Additional context**

_NB: AI-assisted root caus and suggested patch_

`ConversionRecordVisitor::HandleProcDef` fails to traverse the body of a ProcDef's `next()` function with its canonical_initializer.next_type_info, leaving function calls inside next() missing from conversion record collection during IR lowering.

```patch
--- a/xls/dslx/get_conversion_records.cc
+++ b/xls/dslx/get_conversion_records.cc
@@ -424,7 +424,7 @@ class ConversionRecordVisitor : public AstNodeRecursiveVisitor {
(*next_fn)->owner(), canonical_initializer.next_type_info,
include_tests_, proc_id_factory_, top_, resolved_proc_alias_,
records_, processed_invocations_);
- XLS_RETURN_IF_ERROR((*next_fn)->Accept(&next_fn_visitor));
+ XLS_RETURN_IF_ERROR((*next_fn)->body()->Accept(&next_fn_visitor));

XLS_ASSIGN_OR_RETURN(
ConversionRecord cr,
diff --git a/xls/dslx/ir_convert/function_converter.cc b/xls/dslx/ir_convert/function_converter.cc
index 408588b2f..22eb03b5f 100644
--- a/xls/dslx/ir_convert/function_converter.cc
+++ b/xls/dslx/ir_convert/function_converter.cc
@@ -860,6 +860,11 @@ absl::Status FunctionConverter::HandleAllOnesMacro(const AllOnesMacro* node) {

absl::Status FunctionConverter::HandleParam(const Param* node) {
VLOG(5) << "FunctionConverter::HandleParam: " << node->ToString();
+ XLS_ASSIGN_OR_RETURN(Type * dslx_type,
+ current_type_info_->GetItemOrError(node));
+ if (dslx_type->IsProc()) {
+ return absl::OkStatus();
+ }
XLS_ASSIGN_OR_RETURN(xls::Type * type,
ResolveTypeToIr(node->type_annotation()));
Def(node->name_def(), [&](const SourceInfo& loc) {
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.