"Function/proc/block with name not unique within package" Internal error

[mtdudek]

While writing Zstd decoder process I've stumble upon issue with DSLX to IR conversion.
XLS reports redefinition of the name in the package.

E1123 21:08:02.395704 1085018 verifier.cc:1771] INTERNAL: XLS_RET_CHECK failure (xls/ir/verifier.cc:1771) !name_set->contains(function_base->name()) Function/proc/block with name __minimal_example__parse_magic_number__128 is not unique within package minimal_example
0x55b489cf06e0: xabsl::StatusBuilder::CreateStatusAndConditionallyLog()
0x55b489cb4611: xls::VerifyPackage()
0x55b489a28b3f: xls::dslx::(anonymous namespace)::ConvertCallGraph()
0x55b489a26392: xls::dslx::ConvertModuleIntoPackage()
0x55b489a2a730: xls::dslx::ConvertFilesToPackage()
0x55b4899df058: main
0x7f17bb5121ca: [unknown]

I've reduce it to the minimal example that still triggers this behavior.

pub struct Buffer<CAPACITY: u32> {
    content: bits[CAPACITY],
}

pub fn parse_magic_number<CAPACITY: u32>(buffer: Buffer<CAPACITY>) -> Buffer<CAPACITY> {
    buffer
}

struct ZstdDecoderState {
    buffer: Buffer<u32:128>,
}

const ZERO_DECODER_STATE = zero!<ZstdDecoderState>();

pub fn handle_idle_state(state: ZstdDecoderState) -> ZstdDecoderState {
    parse_magic_number<u32:128>(state.buffer);
    state
}

pub proc ZstdDecoder {
    input_r: chan<u32> in;
    output_s: chan<u32> out;

    init {(ZERO_DECODER_STATE)}

    config (
        input_r: chan<u32> in,
        output_s: chan<u32> out,
    ) {(input_r, output_s)}

    next (tok: token, state: ZstdDecoderState) {
        let (tok, _) = recv(tok, input_r);
        handle_idle_state(state);
        let tok = send(tok, output_s, u32:0);
        state
    }
}

@proppy

[mtdudek]

It is not only limited to DSLX to IR conversion. Dslx_interpreter_main also reports the same internal error.

[mtdudek]

I think, I know what is causing this issue.
Moving all function to a separate file allows for DSLX to IR conversion and for interpreter to run tests.
Commenting out parse_magic_number<u32:128>(state.buffer); in the minimal example also works.

I've run minimal example with -v=3 verbosity and logs have following lines:

I1124 10:19:44.153895 1094376 extract_conversion_order.cc:778] Adding to ready sequence: parse_magic_number
I1124 10:19:44.153900 1094376 extract_conversion_order.cc:778] Adding to ready sequence: handle_idle_state
...
I1124 10:19:44.153956 1094376 extract_conversion_order.cc:778] Adding to ready sequence: parse_magic_number
I1124 10:19:44.153959 1094376 extract_conversion_order.cc:778] Adding to ready sequence: handle_idle_state
I1124 10:19:44.153962 1094376 extract_conversion_order.cc:778] Adding to ready sequence: ZstdDecoder.next
I1124 10:19:44.153965 1094376 extract_conversion_order.cc:778] Adding to ready sequence: ZstdDecoder.config

It looks like calling function inside other function causes caller and it callees functions to be translated to IR twice or perhaps more.

[mtdudek]

@proppy I think we can remove blocker, as there is workaround.

[mtdudek]

I've tracked it down to the:
https://github.com/google/xls/blob/main/xls/dslx/ir_convert/extract_conversion_order.cc#L661-L662
RemoveFunctionDuplicates checks if function is parametric and if so, then it is not removed from the ready queue.
I think I may be necessary to check parameters values.

[mtdudek]

I think I have a fix for it, I'll open PR with fix

[proppy]

@mtdudek you mention having the same issue with the interpreter, does it need a patch similar to #1204?

[mtdudek]

Patch in #1204 fixes both interpreter and dslx to IR conversion

[hongted]

This should be fixed due to duplicate issue #1308