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

Extend the fuzzer to check the debug locations output #194

Open
d-sonuga opened this issue Sep 11, 2024 · 7 comments
Open

Extend the fuzzer to check the debug locations output #194

d-sonuga opened this issue Sep 11, 2024 · 7 comments

Comments

@d-sonuga
Copy link
Contributor

The checker only checks for the correctness of the allocation dataflow but not for the debug locations output. Although the correctness of the debug locations output isn’t so critical, I think having it checked will still be a good idea, even if it’s just for testing.

@cfallin
Copy link
Member

cfallin commented Sep 12, 2024

I agree, this would be ideal to check. I don't have time to add it currently; if you want to make an attempt, I'm happy to review though.

@d-sonuga
Copy link
Contributor Author

Yeah, I’ll do it

@Amanieu
Copy link
Contributor

Amanieu commented Sep 14, 2024

Note that debug info as currently implemented is only valid within the instructions themselves but not between original instructions. For example parallel move generation may evict a value to get a scratch register, but this isn't currently reflected in the debug info.

@d-sonuga
Copy link
Contributor Author

@cfallin, @Amanieu, is it correct for there to be multiple entries in the debug_value_labels input with the same label? Because the fuzzer is generating such inputs.

@cfallin
Copy link
Member

cfallin commented Oct 17, 2024

The same label can be attached to different vregs across different ranges of instruction indices, though these shouldn't overlap (i.e. the label should be in only one place at a time).

@d-sonuga
Copy link
Contributor Author

Right - so that means that this:

debug_value_labels: [
        (VReg(vreg = 0, class = Int), Inst(2), Inst(5), 53), 
        (VReg(vreg = 4, class = Int), Inst(2), Inst(8), 53), ]

isn't correct, right? Because the ranges overlap.

@cfallin
Copy link
Member

cfallin commented Oct 18, 2024

That indeed seems invalid! It looks like this loop ensures that over a single instance of the loop, it generates non-contiguous ranges, but it picks an arbitrary label and that label value might have been mentioned in another instance of the loop as well. I guess a fix could be to allocate a new label at that point (though this would prevent labels from moving between vregs, which is interesting to test, but doing better would require some sort of tracking where labels are free).

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