Skip to content

Commit 742017c

Browse files
authored
Widen integer loads and stores to a multiple of 8 bits. (#5986)
This makes Carbon's loads and stores ABI-compatible with Clang's for `bool` and `_BitInt(N)`.
1 parent 3533668 commit 742017c

15 files changed

+182
-58
lines changed

toolchain/lower/function_context.cpp

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -318,6 +318,51 @@ auto FunctionContext::GetReturnTypeInfo(TypeInFile type)
318318
return result;
319319
}
320320

321+
// Given a type used for an LLVM value, return the type that we use to store
322+
// that value in memory. This is the same type unless the type is a
323+
// non-multiple-of-8 integer type, which we explicitly widen to a multiple of 8
324+
// for Clang compatibility and to make our generated IR easier for LLVM to
325+
// handle.
326+
static auto GetWidenedMemoryType(llvm::Type* type) -> llvm::Type* {
327+
if (auto* int_type = dyn_cast<llvm::IntegerType>(type)) {
328+
auto width = llvm::alignToPowerOf2(int_type->getBitWidth(), 8);
329+
if (width != int_type->getBitWidth()) {
330+
return llvm::IntegerType::get(type->getContext(), width);
331+
}
332+
}
333+
return type;
334+
}
335+
336+
auto FunctionContext::LoadObject(TypeInFile type, llvm::Value* addr,
337+
llvm::Twine name) -> llvm::Value* {
338+
auto* llvm_type = GetType(type);
339+
auto* load_type = GetWidenedMemoryType(llvm_type);
340+
341+
// TODO: Include alias and alignment information.
342+
llvm::Value* value = builder().CreateLoad(load_type, addr, name);
343+
344+
if (load_type != llvm_type) {
345+
value = builder().CreateTrunc(value, llvm_type);
346+
}
347+
return value;
348+
}
349+
350+
auto FunctionContext::StoreObject(TypeInFile type, llvm::Value* value,
351+
llvm::Value* addr) -> void {
352+
// TODO: Include alias and alignment information.
353+
auto* llvm_type = GetType(type);
354+
CARBON_CHECK(value->getType() == llvm_type);
355+
356+
// Don't emit a store of `iN` if N is not a multiple of 8. See `LoadObject`.
357+
auto* store_type = GetWidenedMemoryType(llvm_type);
358+
if (store_type != llvm_type) {
359+
// TODO: Should we consider creating a sext if the value is signed?
360+
value = builder().CreateZExt(value, store_type);
361+
}
362+
363+
builder().CreateStore(value, addr);
364+
}
365+
321366
auto FunctionContext::CopyValue(TypeInFile type, SemIR::InstId source_id,
322367
SemIR::InstId dest_id) -> void {
323368
switch (GetValueRepr(type).repr.kind) {
@@ -326,7 +371,7 @@ auto FunctionContext::CopyValue(TypeInFile type, SemIR::InstId source_id,
326371
case SemIR::ValueRepr::None:
327372
break;
328373
case SemIR::ValueRepr::Copy:
329-
builder().CreateStore(GetValue(source_id), GetValue(dest_id));
374+
StoreObject(type, GetValue(source_id), GetValue(dest_id));
330375
break;
331376
case SemIR::ValueRepr::Pointer:
332377
CopyObject(type, source_id, dest_id);

toolchain/lower/function_context.h

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@ class FunctionContext {
6262
struct TypeInFile {
6363
const SemIR::File* file;
6464
SemIR::TypeId type_id;
65+
66+
auto GetPointeeType() -> TypeInFile {
67+
return {.file = file, .type_id = file->GetPointeeType(type_id)};
68+
}
6569
};
6670

6771
// A value representation in a particular file. By convention, this represents
@@ -194,6 +198,14 @@ class FunctionContext {
194198
// Returns the debug location to associate with the specified instruction.
195199
auto GetDebugLoc(SemIR::InstId inst_id) -> llvm::DebugLoc;
196200

201+
// Emits a load of an object representation of type `type`.
202+
auto LoadObject(TypeInFile type, llvm::Value* addr, llvm::Twine name = "")
203+
-> llvm::Value*;
204+
205+
// Emits a store of an object representation of type `type`.
206+
auto StoreObject(TypeInFile type, llvm::Value* value, llvm::Value* addr)
207+
-> void;
208+
197209
// After emitting an initializer `init_id`, finishes performing the
198210
// initialization of `dest_id` from that initializer. This is a no-op if the
199211
// initialization was performed in-place, and otherwise performs a store or a

toolchain/lower/handle_aggregates.cpp

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -103,8 +103,8 @@ static auto GetAggregateElement(FunctionContext& context,
103103
// `elem_ptr` points to a value representation. Load it.
104104
auto result_type = context.GetTypeIdOfInst(result_inst_id);
105105
auto result_value_type = context.GetValueRepr(result_type).type();
106-
return context.builder().CreateLoad(
107-
context.GetType(result_value_type), elem_ptr, name + ".load");
106+
return context.LoadObject(result_value_type, elem_ptr,
107+
name + ".load");
108108
}
109109
case SemIR::ValueRepr::Custom:
110110
CARBON_FATAL(
@@ -265,10 +265,11 @@ static auto EmitAggregateValueRepr(FunctionContext& context,
265265
// Write the value representation to a local alloca so we can produce a
266266
// pointer to it as the value representation of the struct or tuple.
267267
auto* alloca = context.builder().CreateAlloca(llvm_value_rep_type);
268-
for (auto [i, ref] :
268+
for (auto [i, ref_id] :
269269
llvm::enumerate(context.sem_ir().inst_blocks().Get(refs_id))) {
270-
context.builder().CreateStore(
271-
context.GetValue(ref),
270+
context.StoreObject(
271+
context.GetValueRepr(context.GetTypeIdOfInst(ref_id)).type(),
272+
context.GetValue(ref_id),
272273
context.builder().CreateStructGEP(llvm_value_rep_type, alloca, i));
273274
}
274275
return alloca;

toolchain/lower/handle_call.cpp

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -409,17 +409,13 @@ static auto HandleBuiltinCall(FunctionContext& context, SemIR::InstId inst_id,
409409
case SemIR::BuiltinFunctionKind::FloatMulAssign:
410410
case SemIR::BuiltinFunctionKind::FloatDivAssign: {
411411
auto* lhs_ptr = context.GetValue(arg_ids[0]);
412-
auto [lhs_type_file, lhs_type_id] = context.GetTypeIdOfInst(arg_ids[0]);
413-
auto pointee_type_id = lhs_type_file->GetPointeeType(lhs_type_id);
414-
// TODO: Factor out the code to create loads and stores, and include alias
415-
// and alignment information.
416-
auto* lhs_value = context.builder().CreateLoad(
417-
context.GetFileContext(lhs_type_file).GetType(pointee_type_id),
418-
lhs_ptr);
412+
auto lhs_type = context.GetTypeIdOfInst(arg_ids[0]);
413+
auto pointee_type = lhs_type.GetPointeeType();
414+
auto* lhs_value = context.LoadObject(pointee_type, lhs_ptr);
419415
auto* result = CreateBinaryOperatorForBuiltin(
420416
context, inst_id, builtin_kind, lhs_value,
421417
context.GetValue(arg_ids[1]));
422-
context.builder().CreateStore(result, lhs_ptr);
418+
context.StoreObject(pointee_type, result, lhs_ptr);
423419
// TODO: Add a helper to get a "no value representation" value.
424420
context.SetLocal(inst_id,
425421
llvm::PoisonValue::get(context.GetTypeOfInst(inst_id)));
@@ -535,6 +531,7 @@ auto HandleInst(FunctionContext& context, SemIR::InstId inst_id,
535531
// The vtable pointer is always at the start of the object in the Carbon
536532
// ABI, so a pointer to the object is a pointer to the vtable pointer - load
537533
// that to get a pointer to the vtable.
534+
// TODO: Use `context.LoadObject`.
538535
auto* vtable =
539536
context.builder().CreateLoad(ptr_type, args.front(), "vtable");
540537
auto* i32_type = llvm::IntegerType::getInt32Ty(context.llvm_context());

toolchain/lower/handle_expr_category.cpp

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@ auto HandleInst(FunctionContext& context, SemIR::InstId inst_id,
2222
context.SetLocal(inst_id,
2323
llvm::PoisonValue::get(context.GetType(inst_type)));
2424
break;
25-
case SemIR::ValueRepr::Copy: {
26-
auto* type = context.GetType(inst_type);
27-
context.SetLocal(inst_id, context.builder().CreateLoad(
28-
type, context.GetValue(inst.value_id)));
29-
} break;
25+
case SemIR::ValueRepr::Copy:
26+
context.SetLocal(
27+
inst_id,
28+
context.LoadObject(inst_type, context.GetValue(inst.value_id)));
29+
break;
3030
case SemIR::ValueRepr::Pointer:
3131
context.SetLocal(inst_id, context.GetValue(inst.value_id));
3232
break;

toolchain/lower/testdata/array/iterate.carbon

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -90,8 +90,9 @@ fn F() {
9090
// CHECK:STDOUT:
9191
// CHECK:STDOUT: define linkonce_odr i1 @_CHasValue.Optional.Core.b88d1103f417c6d4(ptr %self) !dbg !25 {
9292
// CHECK:STDOUT: %has_value = getelementptr inbounds nuw { i1, i32 }, ptr %self, i32 0, i32 0, !dbg !27
93-
// CHECK:STDOUT: %1 = load i1, ptr %has_value, align 1, !dbg !27
94-
// CHECK:STDOUT: ret i1 %1, !dbg !28
93+
// CHECK:STDOUT: %1 = load i8, ptr %has_value, align 1, !dbg !27
94+
// CHECK:STDOUT: %2 = trunc i8 %1 to i1, !dbg !27
95+
// CHECK:STDOUT: ret i1 %2, !dbg !28
9596
// CHECK:STDOUT: }
9697
// CHECK:STDOUT:
9798
// CHECK:STDOUT: define linkonce_odr i32 @_CGet.Optional.Core.b88d1103f417c6d4(ptr %self) !dbg !29 {
@@ -109,13 +110,13 @@ fn F() {
109110
// CHECK:STDOUT: %has_value = getelementptr inbounds nuw { i1, i32 }, ptr %return, i32 0, i32 0, !dbg !37
110111
// CHECK:STDOUT: %value1 = getelementptr inbounds nuw { i1, i32 }, ptr %return, i32 0, i32 1, !dbg !37
111112
// CHECK:STDOUT: store i32 %value, ptr %value1, align 4, !dbg !37
112-
// CHECK:STDOUT: store i1 true, ptr %has_value, align 1, !dbg !37
113+
// CHECK:STDOUT: store i8 1, ptr %has_value, align 1, !dbg !37
113114
// CHECK:STDOUT: ret void, !dbg !38
114115
// CHECK:STDOUT: }
115116
// CHECK:STDOUT:
116117
// CHECK:STDOUT: define linkonce_odr void @_CNone.Optional.Core.b88d1103f417c6d4(ptr sret({ i1, i32 }) %return) !dbg !39 {
117118
// CHECK:STDOUT: %has_value = getelementptr inbounds nuw { i1, i32 }, ptr %return, i32 0, i32 0, !dbg !40
118-
// CHECK:STDOUT: store i1 false, ptr %has_value, align 1, !dbg !40
119+
// CHECK:STDOUT: store i8 0, ptr %has_value, align 1, !dbg !40
119120
// CHECK:STDOUT: ret void, !dbg !41
120121
// CHECK:STDOUT: }
121122
// CHECK:STDOUT:

toolchain/lower/testdata/builtins/types.carbon

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ fn F() {
5252
// CHECK:STDOUT: call void @llvm.lifetime.start.p0(ptr %h.var), !dbg !11
5353
// CHECK:STDOUT: store fp128 0xL00000000000000003FFF000000000000, ptr %h.var, align 16, !dbg !11
5454
// CHECK:STDOUT: call void @llvm.lifetime.start.p0(ptr %b.var), !dbg !12
55-
// CHECK:STDOUT: store i1 false, ptr %b.var, align 1, !dbg !12
55+
// CHECK:STDOUT: store i8 0, ptr %b.var, align 1, !dbg !12
5656
// CHECK:STDOUT: ret void, !dbg !13
5757
// CHECK:STDOUT: }
5858
// CHECK:STDOUT:

toolchain/lower/testdata/class/generic.carbon

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -301,8 +301,9 @@ fn AccessTuple() -> (i32, i32, i32) {
301301
// CHECK:STDOUT: define linkonce_odr i1 @_CGetBool.C.Main.b88d1103f417c6d4(ptr %self) !dbg !27 {
302302
// CHECK:STDOUT: entry:
303303
// CHECK:STDOUT: %.loc6_16.1.v = getelementptr inbounds nuw { i1, i32 }, ptr %self, i32 0, i32 0, !dbg !28
304-
// CHECK:STDOUT: %.loc6_16.2 = load i1, ptr %.loc6_16.1.v, align 1, !dbg !28
305-
// CHECK:STDOUT: ret i1 %.loc6_16.2, !dbg !29
304+
// CHECK:STDOUT: %.loc6_16.2 = load i8, ptr %.loc6_16.1.v, align 1, !dbg !28
305+
// CHECK:STDOUT: %.loc6_16.21 = trunc i8 %.loc6_16.2 to i1, !dbg !28
306+
// CHECK:STDOUT: ret i1 %.loc6_16.21, !dbg !29
306307
// CHECK:STDOUT: }
307308
// CHECK:STDOUT:
308309
// CHECK:STDOUT: define linkonce_odr i32 @_CGetT.C.Main.b88d1103f417c6d4(ptr %self) !dbg !30 {

toolchain/lower/testdata/for/bindings.carbon

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -92,8 +92,9 @@ fn For() {
9292
// CHECK:STDOUT:
9393
// CHECK:STDOUT: define linkonce_odr i1 @_CHasValue.Optional.Core.29d34654e802e24e(ptr %self) !dbg !18 {
9494
// CHECK:STDOUT: %has_value = getelementptr inbounds nuw { i1, { i32, i32 } }, ptr %self, i32 0, i32 0, !dbg !20
95-
// CHECK:STDOUT: %1 = load i1, ptr %has_value, align 1, !dbg !20
96-
// CHECK:STDOUT: ret i1 %1, !dbg !21
95+
// CHECK:STDOUT: %1 = load i8, ptr %has_value, align 1, !dbg !20
96+
// CHECK:STDOUT: %2 = trunc i8 %1 to i1, !dbg !20
97+
// CHECK:STDOUT: ret i1 %2, !dbg !21
9798
// CHECK:STDOUT: }
9899
// CHECK:STDOUT:
99100
// CHECK:STDOUT: define linkonce_odr void @_CGet.Optional.Core.29d34654e802e24e(ptr sret({ i32, i32 }) %return, ptr %self) !dbg !22 {
@@ -103,7 +104,7 @@ fn For() {
103104
// CHECK:STDOUT:
104105
// CHECK:STDOUT: define linkonce_odr void @_CNone.Optional.Core.29d34654e802e24e(ptr sret({ i1, { i32, i32 } }) %return) !dbg !25 {
105106
// CHECK:STDOUT: %has_value = getelementptr inbounds nuw { i1, { i32, i32 } }, ptr %return, i32 0, i32 0, !dbg !26
106-
// CHECK:STDOUT: store i1 false, ptr %has_value, align 1, !dbg !26
107+
// CHECK:STDOUT: store i8 0, ptr %has_value, align 1, !dbg !26
107108
// CHECK:STDOUT: ret void, !dbg !27
108109
// CHECK:STDOUT: }
109110
// CHECK:STDOUT:

toolchain/lower/testdata/for/break_continue.carbon

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,9 @@ fn For() {
109109
// CHECK:STDOUT:
110110
// CHECK:STDOUT: define linkonce_odr i1 @_CHasValue.Optional.Core.b88d1103f417c6d4(ptr %self) !dbg !34 {
111111
// CHECK:STDOUT: %has_value = getelementptr inbounds nuw { i1, i32 }, ptr %self, i32 0, i32 0, !dbg !36
112-
// CHECK:STDOUT: %1 = load i1, ptr %has_value, align 1, !dbg !36
113-
// CHECK:STDOUT: ret i1 %1, !dbg !37
112+
// CHECK:STDOUT: %1 = load i8, ptr %has_value, align 1, !dbg !36
113+
// CHECK:STDOUT: %2 = trunc i8 %1 to i1, !dbg !36
114+
// CHECK:STDOUT: ret i1 %2, !dbg !37
114115
// CHECK:STDOUT: }
115116
// CHECK:STDOUT:
116117
// CHECK:STDOUT: define linkonce_odr i32 @_CGet.Optional.Core.b88d1103f417c6d4(ptr %self) !dbg !38 {
@@ -130,13 +131,13 @@ fn For() {
130131
// CHECK:STDOUT: %has_value = getelementptr inbounds nuw { i1, i32 }, ptr %return, i32 0, i32 0, !dbg !46
131132
// CHECK:STDOUT: %value1 = getelementptr inbounds nuw { i1, i32 }, ptr %return, i32 0, i32 1, !dbg !46
132133
// CHECK:STDOUT: store i32 %value, ptr %value1, align 4, !dbg !46
133-
// CHECK:STDOUT: store i1 true, ptr %has_value, align 1, !dbg !46
134+
// CHECK:STDOUT: store i8 1, ptr %has_value, align 1, !dbg !46
134135
// CHECK:STDOUT: ret void, !dbg !47
135136
// CHECK:STDOUT: }
136137
// CHECK:STDOUT:
137138
// CHECK:STDOUT: define linkonce_odr void @_CNone.Optional.Core.b88d1103f417c6d4(ptr sret({ i1, i32 }) %return) !dbg !48 {
138139
// CHECK:STDOUT: %has_value = getelementptr inbounds nuw { i1, i32 }, ptr %return, i32 0, i32 0, !dbg !49
139-
// CHECK:STDOUT: store i1 false, ptr %has_value, align 1, !dbg !49
140+
// CHECK:STDOUT: store i8 0, ptr %has_value, align 1, !dbg !49
140141
// CHECK:STDOUT: ret void, !dbg !50
141142
// CHECK:STDOUT: }
142143
// CHECK:STDOUT:

0 commit comments

Comments
 (0)