Internal error during proc inlining

[rw1nkler]

Describe the bug

Inlining a proc that spawns another proc and contains a logic that performs operations on tokens, causes an internal error.

To Reproduce

One can use the following DSLX code to observe the issue:

// DSLX code

proc Passthrough {
    data_r: chan<u32> in;
    data_s: chan<u32> out;

    config(data_r: chan<u32> in, data_s: chan<u32> out) { (data_r, data_s) }

    init { () }

    next(state: ()) {
        let (tok, data) = recv(join(), data_r);
        let tok = send(tok, data_s, data);
    }
}

proc PassthroughNested {
    intr_r: chan<u32> in;
    data_s: chan<u32> out;
    config(data_r: chan<u32> in, data_s: chan<u32> out) {
        let (intr_s, intr_r) = chan<u32, u32:1>("intr");
        spawn Passthrough(data_r, intr_s);
        (intr_r, data_s)
    }

    init { () }
    next(tok: token, state: ()) {
        let (tok, data) = recv(tok, intr_r);
        let tok = send(tok, data_s, data);
    }
}

Here is a bazel rule that can be used to observe the problem:

# BUILD rules

xls_dslx_opt_ir(
    name = "passthrough_nested_opt_ir",
    library = "passthrough_dslx",
    dslx_top = "PassthroughNested",
    opt_ir_args = { "inline_procs": "true" },
)

Building the passthrough_nested_opt_ir target causes the following error:

ERROR: /home/rwinkler/xls/xls/examples/BUILD:1031:16: Optimizing IR file: bazel-out/k8-fastbuild/bin/xls/examples/passthrough_nested_opt_ir.ir failed: (Exit 1): opt_main failed: error executing command (from target //xls/examples:passthrough_nested_opt_ir) bazel-out/k8-opt-exec-2B5CBBC6/bin/xls/tools/opt_main bazel-out/k8-fastbuild/bin/xls/examples/passthrough_nested_opt_ir.ir --inline_procs true --output_path ... (remaining 3 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
E0604 13:45:56.167111   10260 proc_inlining_pass.cc:1388] INTERNAL: XLS_RET_CHECK failure (xls/passes/proc_inlining_pass.cc:1388) !activations_in.empty()
0x559d6610768a: xabsl::StatusBuilder::CreateStatusAndConditionallyLog()
0x559d65f230c9: absl::lts_20240116::StatusOr<>::StatusOr<>()
0x559d65f225e1: xls::(anonymous namespace)::ProcThread::AllocateActivationNode()
0x559d65f1e731: xls::(anonymous namespace)::ProcThread::Create()
0x559d65f0ccca: xls::(anonymous namespace)::InlineProcAsProcThread()
0x559d65f079bd: xls::ProcInliningPass::RunInternal()
0x559d65db9889: xls::PassBase<>::Run()
0x559d65dba882: xls::CompoundPassBase<>::RunNested()
0x559d65dba724: xls::CompoundPassBase<>::RunNested()
0x559d65dba0ae: xls::CompoundPassBase<>::RunInternal()
0x559d65db9889: xls::PassBase<>::Run()
0x559d65db48ac: xls::tools::OptimizeIrForTop()
0x559d65db6677: xls::tools::OptimizeIrForTop()
0x559d65d6d629: main
0x7fc2481ebd90: [unknown]

Error: INTERNAL: XLS_RET_CHECK failure (xls/passes/proc_inlining_pass.cc:1388) !activations_in.empty() ; Running pass #208: Proc inlining [short: proc_inlining]; Running pass #208: Post-inlining passes [short: post-inlining]; Running pass #208: Top level pass pipeline [short: ir]
Target //xls/examples:passthrough_nested_opt_ir failed to build

What is interesting, a procs with empty next(), like the one below, can be inlined without any problems:

proc PassthroughWrapped {
    config(data_r: chan<u32> in, data_s: chan<u32> out) {
        let (intr_s, intr_r) = chan<u32, u32:1>("intr");
        spawn Passthrough(data_r, intr_s);
        spawn Passthrough(intr_r, data_s);
    }

    init { () }
    next(state: ()) { }
}

Expected behavior

It should be possible to inline the procs

[proppy]

I'm not able to reproduce this error w/ v0.0.0-5199-gf59b8a886: https://colab.research.google.com/gist/proppy/b63996aeb0dcbcca7a8271fc322e85c2/xls-playground-sandbox.ipynb

can you confirm which build are you using?

[rw1nkler]

You can observe the problem using the current main. I pushed the code here. Here is the rule that can be used to spot the problem:

bazel build  //xls/examples:passthrough_nested_opt_ir

@proppy

[lpawelcz]

This issue occurs in https://github.com/google/xls/pull/1315 as well. One interesting thing is that we still get errors from the inlining pass even after disabling it in the build rules through "inline_procs": "false" in opt_ir_args (see the CI log). It looks like there is a place in the IR optimization process that includes the inlining pass without checking for the value of the inline_procs flag.

It looks like it might be done here. CreateOptimizationPassPipeline adds the UnrollingAndInliningPassGroup to the optimization passes.

[proppy]

@lpawelcz assuming this is blocking #1211 ?

[proppy]

It looks like there is a place in the IR optimization process that includes the inlining pass without checking for the value of the inline_procs flag

It seems you currently have to remove the flag from opt_ir_args (and not set it to false) in order to disable inlining from a xls_ir_opt_ir target. The handling of this attr in the bzl macro seems lossy, filed #1464.

[lpawelcz]

@lpawelcz assuming this is blocking #1211 ?

@proppy Yes, this is a blocker for the work on ZSTD decoder because we are not able to generate correct verilog code for the modules and run place and route flows.

[rw1nkler]

@hongted it seems that the problem still exists. I updated the example that shows the problem: https://github.com/antmicro/xls/blob/d3ee4f64e4935f939fe096e0d47c6e446474f28e/xls/examples/passthrough.x

[proppy]

@grebe @hongted is there a workaround for this? @rw1nkler is understanding is that inlining is needed for RAM usage.

[proppy]

In order to unblock verilog and PD: I was thinking that they may be able to codegen the memory interface w/o inlining as a regular DSLX proc w/ a channel rdv/vld (without any DSLX ram integration) and stub out the verilog module w/ an adapter to the ram macros; wdyt?

[rw1nkler]

@grebe @hongted is there a workaround for this? @rw1nkler is understanding is that inlining is needed for RAM usage.

The problem is quite specific. So in some cases it is easier to split interaction with RAM between two child processes - one responsible for writing data to RAM, and one for reading if from the memory. In that case we cannot generate a complete Verilog with proper RAM signals, because only half of the interface is available for each child process and the RAM rewriting mechanism will fail. In this particular scenario it is required to inline the proc and allow for rewriting all signal at once. I'm not sure but inlining can also affect the scheduler in that case.

[ericastor]

I believe this is a bug introduced with the new token semantics; the inliner wasn't handling that appropriately. I think I have a fix forthcoming!

[rw1nkler]

It works! Thank you @ericastor