Skip to content

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

Description

@hzeller

Describe the bug

Consider the following code:

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

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

impl MinimalProcTest {
    fn new(done: chan<bool> 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.

--- 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) {

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions