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

Use tbaa for heap-allocated immutables #8867

Merged
merged 1 commit into from
Nov 10, 2014
Merged

Use tbaa for heap-allocated immutables #8867

merged 1 commit into from
Nov 10, 2014

Conversation

simonster
Copy link
Member

This adds tbaa_immut metadata to loads/stores from/to heap-allocated immutables. I'm not entirely sure this is safe, since it is possible for a user to use pointer_from_objref and alter such immutables, but that would seem to go against the spirit of immutables and it's not possible for stack-allocated immutables in any case.

This addresses "hoist access to fields in immutable values" from #3440. It may not be the best we could possibly do, since the field accesses still can't be hoisted in a loop that makes function calls.

On my system, this combined with #8827 is sufficient to make the benchmark from #8809 faster for ContiguousViews than Arrays with @inbounds. (It shouldn't be faster, though. I think this has to do with suboptimal behavior from the LLVM 3.3 loop vectorizer, since the benchmark is twice as fast on an array with LLVM SVN as it is on 3.3.) It gives a speedup without @inbounds but doesn't erase the gap. The IR with bounds checks is very similar; I think the remaining overhead must come from the fact that the bounds checks for ContiguousViews are based on the length of the whole array rather than the length of the view.

@StefanKarpinski
Copy link
Sponsor Member

This is great stuff.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Nov 5, 2014

calling pointer_from_objref on an immutable (isbits) is already inherently a really bad idea (tm), since the memory it returned probably wasn't gc-rooted anywhere. it should probably be disallowed.

@StefanKarpinski
Copy link
Sponsor Member

Let's merge this.

StefanKarpinski added a commit that referenced this pull request Nov 10, 2014
Use tbaa for heap-allocated immutables
@StefanKarpinski StefanKarpinski merged commit 36d1694 into master Nov 10, 2014
@JeffBezanson
Copy link
Sponsor Member

+1

@simonster simonster deleted the sjk/tbaa branch November 10, 2014 16:42
simonster added a commit that referenced this pull request Nov 10, 2014
waTeim pushed a commit to waTeim/julia that referenced this pull request Nov 23, 2014
@simonster
Copy link
Member Author

I wonder whether a more robust approach might be to mark loads from heap-allocated immutables created in other functions as !invariant.load and use llvm.invariant.start for heap-allocated immutables created in the same function (assuming it works, although this LLVM bug suggests it may not). It would be easier to use !invariant.load for immutables created in the same function as well, and the tests pass if I do, but I worry that LLVM could potentially consider the pointer to be "known dereferenceable" before the stores. The discussion here doesn't seem so clear to me.

simonster added a commit that referenced this pull request Dec 12, 2014
Since #8867 this actually makes a difference in the optimizations LLVM
is able to perform.
@JeffBezanson JeffBezanson mentioned this pull request Apr 17, 2015
65 tasks
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.

4 participants