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

brilirs fails on seemingly well-formed program #295

Closed
SanjitBasker opened this issue Sep 15, 2023 · 2 comments
Closed

brilirs fails on seemingly well-formed program #295

SanjitBasker opened this issue Sep 15, 2023 · 2 comments

Comments

@SanjitBasker
Copy link
Contributor

SanjitBasker commented Sep 15, 2023

As part of my dataflow analysis lesson task, I came up with some funky Bril programs to stress the worklist algorithm. The following program (which is easier to understand when the order of the basic blocks is reversed) fails with brilirs -p false false but passes brili -p false false and brilck. It also seems to obey the list of rules given in the Bril documentation.

@main(b0: bool, b1: bool) {
    jmp .start;
  .end:
    print x_0_2;
    print x_1_2;
    ret;
  .l_1_3:
    jmp .end;
  .l_1_2:
    x_1_2 : int = const 0;
    jmp .l_1_3;
  .l_1_1:
    x_1_1 : int = const 1;
    jmp .l_1_3;
  .l_0_3:
    br b1 .l_1_1 .l_1_2;
  .l_0_2:
    x_0_2 : int = const 2;
    jmp .l_0_3;
  .l_0_1:
    x_0_1 : int = const 3;
    jmp .l_0_3;
  .start:
    br b0 .l_0_1 .l_0_2;
}

For reference, here is the Python code I used to generate the program. brilirs has been super useful thus far, but I'm not super familiar with Rust so it'd be nice if a Rustacean could help us out here :)

Pat-Lafon added a commit to Pat-Lafon/bril that referenced this issue Sep 16, 2023
Pat-Lafon added a commit to Pat-Lafon/bril that referenced this issue Sep 16, 2023
@Pat-Lafon Pat-Lafon mentioned this issue Sep 16, 2023
@Pat-Lafon
Copy link
Contributor

Thanks a bunch for report this! It turned out to be a simple fix in brilirs. Non-standard test cases like the one you provided are super helpful. Let me know if you have any more issues.

Try out the fix for yourself, let me know what else you run into(or any random questions). brilirs -t -f test/interp/core/non_linear_control_flow.bril false false

@sampsyo
Copy link
Owner

sampsyo commented Sep 16, 2023

Awesome; many thanks to both of you! @Pat-Lafon beat me to it, but FWIW here is a reduced test case that elicits the same issue:

@main() {
  jmp .start;
.lab:
  ret;
.start:
  jmp .lab;
}

I can confirm that #296 fixes this! A round of applause for @Pat-Lafon. 👏

sampsyo added a commit that referenced this issue Sep 16, 2023
@sampsyo sampsyo closed this as completed Sep 16, 2023
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

3 participants