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

Enable paranoid opt. check for jitdumps resulting from crashes in the optimizer #9386

Closed
fjeremic opened this issue Apr 28, 2020 · 5 comments · Fixed by #11610
Closed

Enable paranoid opt. check for jitdumps resulting from crashes in the optimizer #9386

fjeremic opened this issue Apr 28, 2020 · 5 comments · Fixed by #11610
Labels

Comments

@fjeremic
Copy link
Contributor

I'd like to propose that we enable the TR_EnableParanoidOptCheck option when we are generating a recompilation jitdump resulting from a crash whose VMState determines we are somewhere in the optimizer.

Personally I think this change should be made as I don't see too much of a downside to it, and a large benefit that can potentially identify structural problems found in the trees/CFG before an optimization runs. This can further aid the developers by having an assumption that the tree shapes, reference counts, and the CFG are valid before an optimization starts.

Before making this change I'd like to get input from resident experts in the area of the optimizer for their thoughts, so subscribing @andrewcraik @cathyzhyi @vijaysun-omr for input.

@fjeremic
Copy link
Contributor Author

This issue has been encountered in #8992 where developers requested that paranoid opt checks be enabled for a crash in area of usedef analysis.

@fjeremic
Copy link
Contributor Author

fjeremic commented Jun 2, 2020

@andrewcraik @cathyzhyi @vijaysun-omr any thoughts on this?

@andrewcraik
Copy link
Contributor

It seems a reasonable thing to do from my perspective.

@cathyzhyi
Copy link
Contributor

Sounds reasonable to me as well.

@vijaysun-omr
Copy link
Contributor

Yes, seems like a valid place for paranoidOptCheck to run...it does run the risk of "getting in the way" of the original crash but surely we would'nt have two independent bugs in the same compile and so using the option to catch it as early as possible seems right.

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

Successfully merging a pull request may close this issue.

4 participants