-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Improve generated debug info to avoid assertion in SROA #13762
Conversation
Turns out it just runs through the memory cap trying to optimize the system image ;). |
So the "LLVM bug" is that they didn't buy you more memory ;-p Thanks for the fix. I can confirm that this indeed fixes the bootstrap failure on linux too. |
Not quite. The problem is that bugpoint limits executions of the optimizer to 400MB, the machine has plenty of RAM. |
I remember you said "LLVM does not keep such counts". Is this a recent improvement (that we could use for benchmarking) or is it just running it in a separate process and let the OS take care of it (e.g. with |
Yes, it's using |
Hmm, looks like we have a non-deterministic test failure here :(. Will see if I can reproduce. |
@@ -4569,7 +4569,7 @@ static Function *emit_function(jl_lambda_info_t *lam) | |||
s->name, // Variable name | |||
topfile, // File | |||
toplineno == -1 ? 0 : toplineno, // Line (for now, use lineno of the function) | |||
julia_type_to_di(varinfo.value.typ, ctx.dbuilder, specsig), // Variable type | |||
julia_type_to_di(varinfo.value.typ, ctx.dbuilder, varinfo.value.isboxed), // Variable type |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
isboxed isn't really well defined for varinfo.value, but regardless, it hasn't been assigned yet here. i'm not certain that this is what you want here anyways. I think you want to check after emitting the function whether a memloc needed to be created for this variable, or whether it was allocated in .value
and whether varinfo.value.typ
is a known type or not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm considering always passing false here and handing this at the callsite.
Ok, now passing |
Does this fix #13754? Is it okay to merge? |
This does fix #13754 for me. |
Improve generated debug info to avoid assertion in SROA
This is a very simple start on emitting more thorough debug info. I was planning (and still am) to work on this as part of a larger push to improve our generated debug info, but at the moment SROA is crashing on 3.7 (#13754), because our debug info is not quite valid. Interestingly, I can still see a crash if I run the system image that results from this through bugpoint, the optimizer still crashes, so we might be hitting another LLVM bug here. Bootstrap goes through fine with this though.