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

Test has incorrect lowering #410

Open
Mark1626 opened this issue Jan 22, 2024 · 3 comments
Open

Test has incorrect lowering #410

Mark1626 opened this issue Jan 22, 2024 · 3 comments

Comments

@Mark1626
Copy link
Collaborator

The test file-tests/should-lower/hoist.fuse fails typecheck as there is an out of bounds access let w = b[10 * 12];. However when --lower is present it seems to pass.

decl a: bit<32>[1];
decl b: bit<32>[1];

// Recusively hoist nested reads
let x = a[b[0]];
---

// Hoist binary expressions with reads
let y = 10 + a[0];

// Recusively hoist slow ops
let z = (x * 10) / (y * 20 - 15);

// Recursively hoist slow index expressions
let w = b[10 * 12];

// Complex chained binary expression
let k = (((0-1) * 1 + (2*2) * 2 - 1*1 - 1*2) as ubit<32>);

Error without --lower

[WARN] [5.11] bit<32> is used for an array access. This might be out of bounds at runtime.
let x = a[b[0]];
          ^

[Type error] [Line 15, Column 11] Index out of bounds for `b'. Memory size is 1, iterator max val is 120
let w = b[10 * 12];
          ^
@rachitnigam
Copy link
Member

That's odd! Lowering should still go through the type checker. Can you try using --log info or --log debug to see what's going wrong? Maybe a type checking pass is not being correctly run. FWIW, bounds checking is not a part of the type checker and instead a different pass so possible that the logic is wrong and we miss that pass

@Mark1626
Copy link
Collaborator Author

Issue is in the typecheck. I'll see if I can find a fix

Error from --log debug

[INFO] fuselang.passes.BoundsChecker$BCheck$$anonfun$myCheckE$1$$anonfun$applyOrElse$3.$anonfun$applyOrElse$6(BoundsCheck.scala:84)
fuselang.passes.BoundsChecker$BCheck$$anonfun$myCheckE$1$$anonfun$applyOrElse$3.$anonfun$applyOrElse$6$adapted(BoundsCheck.scala:71)
scala.Option.foreach(Option.scala:437)
fuselang.passes.BoundsChecker$BCheck$$anonfun$myCheckE$1$$anonfun$applyOrElse$3.$anonfun$applyOrElse$5(BoundsCheck.scala:71)
fuselang.passes.BoundsChecker$BCheck$$anonfun$myCheckE$1$$anonfun$applyOrElse$3.$anonfun$applyOrElse$5$adapted(BoundsCheck.scala:69)
scala.collection.immutable.List.foreach(List.scala:333)
fuselang.passes.BoundsChecker$BCheck$$anonfun$myCheckE$1$$anonfun$applyOrElse$3.applyOrElse(BoundsCheck.scala:69)
fuselang.passes.BoundsChecker$BCheck$$anonfun$myCheckE$1$$anonfun$applyOrElse$3.applyOrElse(BoundsCheck.scala:64)
scala.PartialFunction$OrElse.apply(PartialFunction.scala:266)
fuselang.common.Syntax$RichType.matchOrError(Syntax.scala:327)

[Type error] [Line 15, Column 11] Index out of bounds for `b'. Memory size is 1, iterator max val is 120
let w = b[10 * 12];
          ^

Bounds check can be bypassed if the index is stored in a variable

Minimal example

The following fails as expected

decl b: bit<32>[1];

let x = b[10 * 12];

This passes when it should not

decl b: bit<32>[1];

let idx = 10 * 12;
let x = b[idx];

In the test --lower raised the constant index to a variable

The final generated HLS code

    ap_int<7> bin_read3_ = ((ap_int<7>)10 * (ap_int<7>)12);
    //---
    ap_int<32> w_ = b[(ap_uint<1>)bin_read3_];

@Mark1626
Copy link
Collaborator Author

I guess this case could be considered valid.

decl b: bit<32>[1];

let idx = 10 * 12;
let x = b[idx];

It's possible for a variable to change

decl b: bit<32>[1];

let idx = 10 * 12;
let x = b[idx];
---
idx := 1

It can be inlined if either if it's marked as a constant or a compilation pass inlines it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants