Emit more assumptions in trans #36962
Conversation
|
r? @eddyb |
| pub fn load_fat_ptr(b: &Builder, fat_ptr: ValueRef) -> (ValueRef, ValueRef) { | ||
| (b.load(get_dataptr(b, fat_ptr)), b.load(get_meta(b, fat_ptr))) | ||
| } | ||
|
|
There was a problem hiding this comment.
I prefer moving things to MIR, not away from it, TBH.
There was a problem hiding this comment.
But then non-MIR would have to call into it. I mostly want the load/store primitives all in one place.
There was a problem hiding this comment.
Alright, we'll move them when we get rid of all non-MIR code.
There was a problem hiding this comment.
TBH, I would prefer that everything will be done through OperandRef and be in 1 file. That would require some more refactoring through.
There was a problem hiding this comment.
I like the current structure where rvalue/block/stmt/lvalue are rather independent of each-other, and all "utility" methods go into a different place.
|
@bors r=eddyb |
|
📌 Commit 74447d8 has been approved by |
|
So, I recall we held back on adding assume to a lot of places cause it slowed down LLVM a bit. I'm not sure how much more efficient that's become? cc @dotdash |
|
C-enum-to-integer casts are not really a common codepath - through we should see that this doesn't hit some terrible O(n^2) case with serialization or something. |
|
☔ The latest upstream changes (presumably #36958) made this pull request unmergeable. Please resolve the merge conflicts. |
cc rust-lang#36920 (in addition to LLVM PR30597, should fix the &&[i32] case)
arielb1
force-pushed
the
cast-assumptions
branch
from
74447d8
to
45fe3a1
Compare
|
@bors r=eddyb |
|
📌 Commit 45fe3a1 has been approved by |
Emit more assumptions in trans Perf numbers pending.
Eliminate some other bound checks when index comes from an enum rust-lang#36962 introduced an assumption for the upper limit of the enum's value. This PR adds an assumption to the lower value as well. I've modified the original codegen test to show that derived (in that case, adding 1) values also don't generate bounds checks. However, this test is actually carefully crafted to not hit a bug: if the enum's variants are modified to 1 and 2 instead of 2 and 3, the test fails by adding a bounds check. I suppose this is an LLVM issue and rust-lang#75525, while not exactly in this context should be tracking it. I'm not at all confident if this patch can be accepted, or even if it _should_ be accepted in this state. But I'm curious about what others think :) ~Improves~ Should improve rust-lang#13926 but does not close it because it's not exactly predictable, where bounds checks may pop up against the assumptions.
Perf numbers pending.