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

lineno fixes #13491

Merged
merged 2 commits into from
Oct 8, 2015
Merged

lineno fixes #13491

merged 2 commits into from
Oct 8, 2015

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Oct 7, 2015

fixes #922 (wrong line numbers in error expressions)

{
jl_throw(e);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:-(

@ihnorton
Copy link
Member

ihnorton commented Oct 8, 2015

👍 LGTM

@Keno
Copy link
Member

Keno commented Oct 8, 2015

Are we sure that disabling tail merging globally is a good idea? What are the performance impacts on other code that benefits from this optimization?

@vtjnash
Copy link
Member Author

vtjnash commented Oct 8, 2015

Are we sure that disabling tail merging globally is a good idea? What are the performance impacts on other code that benefits from this optimization?

it should result in a very small performance increase (yes, benefit) on some paths in functions that this affects, at the cost of not detecting when the user has duplicated code on multiple function branches (e.g. due to inlining and macros) and folding away the line numbers for that (thus losing on the code-size benefit of this optimization).

we might want to manually merge all of the gc-frame pops into a single BasicBlock to cut down on the number of prologues that get emitted. (e.g. this pass, but with more specificity for Julia)

vtjnash added a commit that referenced this pull request Oct 8, 2015
fix tail line numbers in backtraces
@vtjnash vtjnash merged commit 646ea37 into master Oct 8, 2015
@vtjnash vtjnash deleted the jn/lineno_fixes branch October 8, 2015 15:35
@vtjnash
Copy link
Member Author

vtjnash commented Oct 9, 2015

also include a70d7cf when backporting

@tkelman
Copy link
Contributor

tkelman commented Dec 2, 2015

removing the backport label given #14186

tkelman pushed a commit that referenced this pull request Mar 13, 2016
…ks around this issue

(cherry picked from commit af81d6c)
ref #13491
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

Successfully merging this pull request may close these issues.

incorrect line number reported for index out of range error
6 participants