-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Extra jumps + instrs for exceptions #3354
Comments
I think even without any of the branch condition changes, a throw in a scope that gets caught unconditionally by a catch in the same scope could be replaced with a jump ... but now that I said it, I think it will require BB 10 to be split into 2 since recv_jruby_exc in BB10 will no longer apply without the throw. Anyway recording this for posterity if this ever shows up as a perf issue. |
@subbuss I think perhaps we should also consider the rescues above (it will not end up changing your conclusion about splitting 10 unfortunately):
The optimization would be to remove 7 so then only 6 and 8 can throw to 10. Can 8 really throw? It may be worth it to consider this FIXME you made:
I do wonder how much f_can_raise_exception is defeating some optimizations. I guess calls are so frequent in Ruby code perhaps it does not matter a ton, but in this case I do not think $! need to worry about throwing. With potentially a set_global_safe instr we can eliminate 6 from needing a rescue to 10. Secondly, I think we could consider that we might have also uncovered a semantic bug in 6. Consider:
Note in IR we end up exposing %undefined and in MRI it works. fwiw, NO ONE will be removing StandardError but I think we should be getting a pre-saved version of StandardError for this case (oh no another instr :) ). The only other instr which can raise is rescue_eqq. I proved to myself that we can actually raise in this one:
This is a bummer and I guess means we have to split 10. In spite of all this work to realize 10 needs to be split I found a bug in 6 and I think we need to review constants we retrieve to see how many cannot be overriden in user code. Retrieving those with a new instr could also be a perf gain (for that matter %StandardError would not be a bad idea). |
Reg. StandardError, yes, it is a known issue -- see this FIXME from IRBuilder. :-)
|
@subbuss haha...IR knows all problems if you read the source long enough :) |
If we look at a very simple snippet in IR:
Here is IR of last normal pass:
Notice in an ideal world BB6 could fall through to BB8 and BB7 could be deleted and the b_true in BB6 could become a b_false and go straight to BB 10. This would end up eliminating an extra BB and also not force us to throw an extra time. @subbuss said he thought perhaps this can be post pass on CFG construction since it can happen in more conditions than this single rescue form.
The text was updated successfully, but these errors were encountered: