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

fix effect_free computation over getfield #18017

Merged
merged 1 commit into from
Aug 15, 2016
Merged

Conversation

vtjnash
Copy link
Member

@vtjnash vtjnash commented Aug 13, 2016

fix #18015

@JeffBezanson this also fixes the bug you noted where inlineable was passing the wrong sv to effect_free. It would be appreciated if you could verify I've picked the right linfo to pass in each case.

@tkelman
Copy link
Contributor

tkelman commented Aug 13, 2016

osx travis got stuck, log backed up to https://gist.github.com/a62ec9f360ea4b4c8a8cdaec465b52dd and restarted

# first argument must be immutable to ensure e is affect_free
a = ea[2]
typ = widenconst(exprtype(a, linfo))
if !isa(typ, DataType) || typ.mutable
Copy link
Member

Choose a reason for hiding this comment

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

I think we might need || typ.abstract here.

@JeffBezanson
Copy link
Member

if you could verify I've picked the right linfo to pass in each case

Yes that part looks right to me.

@JeffBezanson JeffBezanson added backport pending 0.5 bugfix This change fixes an existing bug labels Aug 14, 2016
@vtjnash vtjnash merged commit 1c70092 into master Aug 15, 2016
@vtjnash vtjnash deleted the jn/effect-free-getfield branch August 15, 2016 18:40
tkelman referenced this pull request Aug 16, 2016
Expand docs on enumerate's relationship with indexing
@KristofferC
Copy link
Member

Can we try to use this super awesome benchmark tool a bit more frequently. A run doesn't take that long. Saves @tkelman from manually having to bisect performance regressions.

@JeffBezanson
Copy link
Member

I agree. Running benchmarks on every commit is starting to get urgent. If it takes too long or we don't have enough resources, we should run a reduced version of the suite. Running on every commit is much more important than having perfect coverage.

JeffBezanson added a commit that referenced this pull request Aug 18, 2016
tkelman pushed a commit that referenced this pull request Aug 20, 2016
tkelman pushed a commit that referenced this pull request Aug 20, 2016
fixes regression caused by #18017

(cherry picked from commit e35a7f6)
ref #18058
tkelman added a commit that referenced this pull request Aug 21, 2016
tkelman pushed a commit that referenced this pull request Aug 21, 2016
(cherry picked from commit b32fcc9)
ref #18126
tkelman referenced this pull request Aug 22, 2016
required for the inliner to work, since all of the logic for computing edges is supposed to be handle by the inference step

fix #17759

(cherry picked from commit 19b1276)
ref #17949
mfasi pushed a commit to mfasi/julia that referenced this pull request Sep 5, 2016
mfasi pushed a commit to mfasi/julia that referenced this pull request Sep 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This change fixes an existing bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inlining(?) bug on 0.5
4 participants