Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DSLX: bug: internal error when using derived parametric #992

Closed
rdob-ant opened this issue May 29, 2023 · 4 comments
Closed

DSLX: bug: internal error when using derived parametric #992

rdob-ant opened this issue May 29, 2023 · 4 comments
Assignees
Labels
bug Something isn't working or is incorrect dslx DSLX (domain specific language) implementation / front-end duplicate This issue or pull request already exists
Milestone

Comments

@rdob-ant
Copy link
Contributor

Consider following parametric structure:

struct MyStruct<K:u32, N: u32, M: u32 = {u32:1<<N}> {
    foo: uN[K][M],
}

Sample 1

fn genpass1<K: u32, N: u32>(arg: MyStruct<K, N>) -> MyStruct<K, N> {
    arg
}

fn genfunc1<K: u32, N: u32>() -> MyStruct<K, N> {
    let x = MyStruct<K, N> {
        foo: uN[K][u32:1<<N]:[uN[K]:0, ...],
    };
    genpass1<K, N>(x)
}

fn func1() -> MyStruct<3, 4> {
    genfunc1<u32:3, u32:4>()
}

This code generates following internal error:

E0529 20:05:09.279610       3 command_line_utils.cc:46] Could not extract a textual position from error message: INTERNAL: XLS_RET_CHECK failure (xls/dslx/type_system/parametric_bind.cc:97) ctx.parametric_binding_types.contains(pdim_name) Cannot bind M : it has no associated type.: INVALID_ARGUMENT: Provided status is not in recognized error form: INTERNAL: XLS_RET_CHECK failure (xls/dslx/type_system/parametric_bind.cc:97) ctx.parametric_binding_types.contains(pdim_name) Cannot bind M : it has no associated type.
F0529 20:05:09.279666       3 interpreter_main.cc:183] Check failed: ::absl::OkStatus() == (status) (OK vs. INTERNAL: XLS_RET_CHECK failure (xls/dslx/type_system/parametric_bind.cc:97) ctx.parametric_binding_types.contains(pdim_name) Cannot bind M : it has no associated type.)
Error parsing and type checking DSLX source file

Sample 2

proc genproc2<K: u32, N: u32> {
    init {
        MyStruct<K, N> {
            foo: uN[K][u32:1<<N]:[uN[K]:0, ...],
        }
    }

    config(dummy: chan<u8> out) {
        ()
    }

    next(tok: token, state: MyStruct<K, N>) {
        state
    }
}

proc proc2 {
    init {()}

    config(dummy: chan<u8> out) {
        spawn genproc2<u32:3, u32:4>(dummy);
        ()
    }

    next(tok: token, st: ()) {()}
}

Contrary to sample 1, this results in the following type error:

0044:   proc genproc2<K: u32, N: u32> {
0045:       init {
       _________^
0046: |         MyStruct<K, N> {
0047: |             foo: uN[K][u32:1<<N]:[uN[K]:0, ...],
0048: |         }
0049: |     }
      |_____^ XlsTypeError: struct 'MyStruct' structure: MyStruct { foo: uN[3][16] } vs struct 'MyStruct' structure: MyStruct { foo: uN[3][M] }: 'next' state param and 'init' types differ.
0050:
0051:       config(dummy: chan<u8> out) {

-----

The first one is surely a bug as it raises internal error; the second one I am not sure, but as a user I would prefer it to just work, since I think the code contains enough information for the interpreter to resolve all the parametrics (unless I'm missing something!)

@proppy proppy added dslx DSLX (domain specific language) implementation / front-end bug Something isn't working or is incorrect labels May 30, 2023
@proppy
Copy link
Member

proppy commented May 30, 2023

@rdob-ant is this blocking #994 or #995?

@rdob-ant
Copy link
Contributor Author

@proppy I haven't encountered Sample 1 yet when working on RLE/DBE. Sample 2 affected me when writing DBE code, but thankfully we have a workaround for this (namely, to use a derived parametric in the proc which instantiates the struct), so it's not a blocker.

@mtdudek mtdudek moved this to In Progress in Antmicro XLS May 31, 2023
@hongted hongted added this to the DSLX Next milestone Aug 5, 2024
@proppy proppy moved this to Todo in XLS usability Aug 20, 2024
@erinzmoore erinzmoore self-assigned this Sep 5, 2024
@m-torhan
Copy link

m-torhan commented Sep 6, 2024

I recently got similar issue when trying to create parametrized struct with derived parametrics, however I got different error message (possibly because there were changes in code since this one was opened).

Sample code:

struct StructFoo<A: u32, B: u32 = {u32:32}, C:u32 = {B / u32:2}> {
    a: uN[A],
    b: uN[B],
    c: uN[C],
}

fn FuncFoo<A: u32, B: u32 = {u32:32}, C:u32 = {B / u32:2}>() -> () {
    trace_fmt!("func A {}", A);
    trace_fmt!("func B {}", B);
    trace_fmt!("func C {}", C);
}

fn print_struct_widths<A: u32, B: u32, C: u32>(s: StructFoo<A, B, C>) -> () {
    trace_fmt!("struct A {}", A);
    trace_fmt!("struct B {}", B);
    trace_fmt!("struct C {}", C);
}

#[test]
fn Foo() {
    FuncFoo<u32:32, u32:16>();
    print_struct_widths(zero!<StructFoo<u32:32, u32:16>>());

    FuncFoo<u32:32>();
    print_struct_widths(zero!<StructFoo<u32:32>>());

    FuncFoo<u32:32, u32:16, u32:16>();
    print_struct_widths(zero!<StructFoo<u32:32, u32:16, u32:16>>());
}

Error after building dslx target:

E0906 10:26:22.402447  660988 command_line_utils.cc:46] Could not extract a textual position from error message: INVALID_ARGUMENT: Cannot convert expression to parametric: B / u32:2: INVALID_ARGUMENT: Provided status is not in recognized error form: INVALID_ARGUMENT: Cannot convert expression to parametric: B / u32:2
Error parsing and type checking DSLX source file: foo.x
Error: INVALID_ARGUMENT: Cannot convert expression to parametric: B / u32:2Target //:foo_dslx_test failed to build

I expect the struct's parameters to behave similarly to function's parameters, so I added there the parametric function to compare the results.

Also I noticed that when there is no default value set for parametric that is used to derive another parametric, then the error is returned. For example, if we change FuncFoo definition to

fn FuncFoo<A: u32, B: u32, C:u32 = {B / u32:2}>() -> () {
    trace_fmt!("func A {}", A);
    trace_fmt!("func B {}", B);
    trace_fmt!("func C {}", C);
}

then we get

foo.x:7:37-7:38
0005: }
0006:
0007: fn FuncFoo<A: u32, B: u32, C:u32 = {B / u32:2}>() -> () {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^ TypeInferenceError: Parametric expression `B / u32:2` refered to `B` which is not present in the parametric environment; instantiated from foo.x:24:12-24:22
0008:     trace_fmt!("func A {}", A);
0009:     trace_fmt!("func B {}", B);

XLS commit hash: 3f7a3d1cb0c8d3403b3a6541ca1aa44cf41401dd

copybara-service bot pushed a commit that referenced this issue Sep 10, 2024
copybara-service bot pushed a commit that referenced this issue Sep 11, 2024
For structs, these only need to be set once the struct is actually used. There's a separate check for that.

#992

PiperOrigin-RevId: 673378184
@erinzmoore
Copy link
Collaborator

erinzmoore commented Sep 11, 2024

I wasn't able to exactly reproduce the original issue here, but investigating this comment exposed a couple of bugs related to:

  • supported operations in ParametricExpression
  • using non-defaulted vars in struct parametric expressions

The second item should be fixed by 6cf8a0d. For the first item, I opened #1597 as a longer-term project. If there are specific operations/expressions that are missing and blocking work, please let us know and we can prioritize those.

@erinzmoore erinzmoore closed this as not planned Won't fix, can't repro, duplicate, stale Sep 11, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in Antmicro XLS Sep 11, 2024
@github-project-automation github-project-automation bot moved this from In Progress to Done in XLS usability Sep 11, 2024
@erinzmoore erinzmoore added the duplicate This issue or pull request already exists label Sep 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working or is incorrect dslx DSLX (domain specific language) implementation / front-end duplicate This issue or pull request already exists
Projects
Status: Done
Development

No branches or pull requests

5 participants