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

Removes exception handler inlinining from -optimize #3159

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
5 participants
@JamesIry
Contributor

JamesIry commented Nov 19, 2013

Exception handler inlining is an optimization that’s hard to get right
(and in fact has open bugs) but that has questionable benefits. This
commit removes -inlineHandlers from being included under -optimize. If
there are no serious repercussions then the optimization may be entirely
removed later.

James Iry
Removes exception handler inlinining from -optimize
Exception handler inlining is an optimization that’s hard to get right
(and in fact has open bugs) but that has questionable benefits. This
commit removes -inlineHandlers from being included under -optimize. If
there are no serious repercussions then the optimization may be entirely
removed later.
@som-snytt

This comment has been minimized.

Show comment
Hide comment
@som-snytt

som-snytt Nov 19, 2013

Oops, can't just --update-check when there are conditionals. That's a current limitation, quite annoying.

Oops, can't just --update-check when there are conditionals. That's a current limitation, quite annoying.

@dragos

This comment has been minimized.

Show comment
Hide comment
@dragos

dragos Nov 21, 2013

Contributor

I think it's still useful (for instance after inlining breakable/break). See #3154. There are open bugs in many areas of the compiler, is there a particular reason why this feature is being singled out for removal? @VladUreche might help with bugs ;-)

(edit) @JamesIry replied in #3154.

Contributor

dragos commented Nov 21, 2013

I think it's still useful (for instance after inlining breakable/break). See #3154. There are open bugs in many areas of the compiler, is there a particular reason why this feature is being singled out for removal? @VladUreche might help with bugs ;-)

(edit) @JamesIry replied in #3154.

@VladUreche

This comment has been minimized.

Show comment
Hide comment
@VladUreche

VladUreche Nov 21, 2013

Member

@dragos, I'm not insisting on either keeping or enabling inlineEH since I know the entire backend will be superseded by Miguel's backend.
@JamesIry, I found SI-7792 and SI-7589: https://issues.scala-lang.org/browse/SI-5290?jql=status%20%3D%20Open%20AND%20text%20~%20%22\%22-Yinline-handlers\%22%22, do you have others?

Member

VladUreche commented Nov 21, 2013

@dragos, I'm not insisting on either keeping or enabling inlineEH since I know the entire backend will be superseded by Miguel's backend.
@JamesIry, I found SI-7792 and SI-7589: https://issues.scala-lang.org/browse/SI-5290?jql=status%20%3D%20Open%20AND%20text%20~%20%22\%22-Yinline-handlers\%22%22, do you have others?

@adriaanm

This comment has been minimized.

Show comment
Hide comment
@adriaanm

adriaanm Nov 25, 2013

Member

So, unless there are any bad bugs due to exception inlining, it sounds like this PR should be closed? (As we have a good use case -- breakable -- that's described in the PR referenced by Iulian.)

Member

adriaanm commented Nov 25, 2013

So, unless there are any bad bugs due to exception inlining, it sounds like this PR should be closed? (As we have a good use case -- breakable -- that's described in the PR referenced by Iulian.)

@JamesIry JamesIry closed this Nov 26, 2013

magarciaEPFL added a commit to magarciaEPFL/scala that referenced this pull request Dec 7, 2013

remove InlineExceptionHandlers from the compiler
The main use case for this optimization is
leaner control flow upon inlining breakable/break,
as discussed in scala#3159.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment