Skip to content

Properly translate boolean statics to be stored as i8 #17875

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

Merged
merged 1 commit into from
Oct 9, 2014
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/librustc/middle/trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2680,12 +2680,18 @@ pub fn get_item_val(ccx: &CrateContext, id: ast::NodeId) -> ValueRef {

// We need the translated value here, because for enums the
// LLVM type is not fully determined by the Rust type.
let (v, inlineable, _) = consts::const_expr(ccx, &**expr, is_local);
let (v, inlineable, ty) = consts::const_expr(ccx, &**expr, is_local);
ccx.const_values().borrow_mut().insert(id, v);
let mut inlineable = inlineable;

unsafe {
let llty = llvm::LLVMTypeOf(v);
// boolean SSA values are i1, but they have to be stored in i8 slots,
// otherwise some LLVM optimization passes don't work as expected
let llty = if ty::type_is_bool(ty) {
llvm::LLVMInt8TypeInContext(ccx.llcx())
} else {
llvm::LLVMTypeOf(v)
};
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a comment about what translation is going on here?

if contains_null(sym.as_slice()) {
ccx.sess().fatal(
format!("Illegal null byte in export_name value: `{}`",
Expand Down
7 changes: 7 additions & 0 deletions src/librustc/middle/trans/consts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -701,6 +701,13 @@ pub fn trans_const(ccx: &CrateContext, m: ast::Mutability, id: ast::NodeId) {
// At this point, get_item_val has already translated the
// constant's initializer to determine its LLVM type.
let v = ccx.const_values().borrow().get_copy(&id);
// boolean SSA values are i1, but they have to be stored in i8 slots,
// otherwise some LLVM optimization passes don't work as expected
let v = if llvm::LLVMTypeOf(v) == Type::i1(ccx).to_ref() {
llvm::LLVMConstZExt(v, Type::i8(ccx).to_ref())
} else {
v
};
Copy link
Member

Choose a reason for hiding this comment

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

Could you also add a comment about why it's being extended here?

llvm::LLVMSetInitializer(g, v);

// `get_item_val` left `g` with external linkage, but we just set an
Expand Down