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

Parametric signedness bit type doesn't work #1572

Closed
mikex-oss opened this issue Aug 29, 2024 · 2 comments · Fixed by #1606
Closed

Parametric signedness bit type doesn't work #1572

mikex-oss opened this issue Aug 29, 2024 · 2 comments · Fixed by #1606
Assignees
Labels
bug Something isn't working or is incorrect dslx DSLX (domain specific language) implementation / front-end

Comments

@mikex-oss
Copy link
Collaborator

Describe the bug
The bytecode emitter errors out if you try to use a return type like xN[RESULT_SIGNED][RESULT_SZ].

: INVALID_ARGUMENT: Provided status is not in recognized error form: INTERNAL: Bits-to-array cast bit counts must match. Saw 32 vs 0.; while evaluating: to_int<u32:8, u32:23, u32:32>(APFloat<u32:8, u32:23> { sign: u1:0, bexp: u8:0x0, fraction: u23:0x0 })
...
Error: INTERNAL: Bits-to-array cast bit counts must match. Saw 32 vs 0.; while evaluating: ...
=== Source Location Trace: ===

To Reproduce
Steps to reproduce the behavior:
Change this line:

(x: APFloat<EXP_SZ, FRACTION_SZ>) -> bits[RESULT_SZ] {

to xN[RESULT_SIGNED][RESULT_SZ] (updating the casts at

result as bits[RESULT_SZ]
and
result as bits[RESULT_SZ]
)

Expected behavior
This should properly include parameterized signedness and remove the need to cast the result in the wrapper at

to_signed_or_unsigned_int<RESULT_SZ, true>(x) as sN[RESULT_SZ]

@mikex-oss mikex-oss added bug Something isn't working or is incorrect dslx DSLX (domain specific language) implementation / front-end labels Aug 29, 2024
@mikex-oss
Copy link
Collaborator Author

@proppy suggested this undocumented feature, pointing to 9761efb.

Once fixed, it would also be good to document it at https://google.github.io/xls/dslx_reference/#bit-type. xN is referenced in a couple places, but it's not clear this is not just a convenience shorthand for documentation.

@cdleary cdleary self-assigned this Sep 13, 2024
@cdleary
Copy link
Collaborator

cdleary commented Sep 13, 2024

Note this test case exhibits the same problem:

#[test]
fn casts() {
    assert_eq(s1:1, u1:1 as xN[true][1]);
    assert_eq(u1:1, s1:1 as xN[false][1]);
}

#[test]
fn widening_casts() {
    assert_eq(s2:0b11, s1:1 as xN[true][2]);
    assert_eq(u2:0b01, u1:1 as xN[false][2]);

    assert_eq(s2:0b11, xN[true][1]:1 as s2);
    assert_eq(u2:0b11, xN[false][1]:1 as u2);
}

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
Projects
Status: Done
2 participants