-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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
Pass around i1 as is instead of going through u8 #50277
Conversation
(rust_highfive has picked a reviewer for you, use r? to override) |
Cc @rkruppe |
Pass around i1 as is instead of going through u8
let v = base::from_immediate(&bx, v); | ||
if common::val_ty(v) == Type::i1(bx.cx) { | ||
v = bx.zext(v, Type::i8(bx.cx)) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am not sure about this, but I feel like boolean arrays should also use memset
when they can, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed.
☀️ Test successful - status-travis |
cc @Mark-Simulacrum We're curious about a perf run here. |
Perf queued. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
I can't reproduce that locally, I wonder if this is an intermittent, or a failure specific to using LLVM 3.9. |
Seems like this was indeed an intermittent. |
@bors r+ |
📌 Commit d605b3d has been approved by |
src/librustc_trans/type_of.rs
Outdated
@@ -359,7 +352,7 @@ impl<'tcx> LayoutLlvmExt<'tcx> for TyLayout<'tcx> { | |||
}; | |||
let scalar = [a, b][index]; | |||
|
|||
// Make sure to return the same type `immediate_llvm_type` would, | |||
// Make sure to return the same type `llvm_type` would, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, looking at this again, the special-case for bool
could go better in scalar_llvm_type_at
, which would let you remove this whole comment too.
@bors r+ |
📌 Commit 4eb72a5 has been approved by |
⌛ Testing commit 4eb72a58711d6a9c587e2a76b231b9bb5fcb69d3 with merge d14b569c45bf4364eb727488157076212597fdc4... |
💔 Test failed - status-travis |
@bors r+ |
📌 Commit 1c95afc has been approved by |
⌛ Testing commit 1c95afcbb6d2e4efad703a71fab624fa0ff34af9 with merge 15b212b19433edee1f61bcf6e85516d08020398a... |
💔 Test failed - status-appveyor |
This comment has been minimized.
This comment has been minimized.
@bors r+ |
📌 Commit 0958407 has been approved by |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
@bors r- ↑ |
Ping from triage @nox! It's been a while since we heard from you, will you have time to work on this again soon? |
☔ The latest upstream changes (presumably #50249) made this pull request unmergeable. Please resolve the merge conflicts. |
Ping from triage @nox! It's been a while since we heard from you, will you have time to work on this again soon? |
No description provided.