Skip to content

Commit

Permalink
Fix incorrect lowering of mixed constant / non-constant aggregate ini…
Browse files Browse the repository at this point in the history 
…tialization. (#4704)

We defer lowering initialization with constant values, because the use
of the constant can itself be part of a larger constant that we don't
want to emit. However, when the initialization is for an element of a
non-constant aggregate, we do need to initialize the constant portion.
  • Loading branch information
@zygoloid
zygoloid authored Dec 19, 2024
1 parent ee4746c commit b54a27e
Show file tree
Hide file tree
Showing 4 changed files with 165 additions and 2 deletions.
97 changes: 97 additions & 0 deletions toolchain/check/testdata/struct/partially_const.carbon
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
// Exceptions. See /LICENSE for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
// AUTOUPDATE
// TIP: To test this file alone, run:
// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/check/testdata/struct/partially_const.carbon
// TIP: To dump output, run:
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/check/testdata/struct/partially_const.carbon

fn Make(n: i32) -> {.a: i32, .b: i32, .c: i32} {
return {.a = 0, .b = n, .c = 0};
}

// CHECK:STDOUT: --- partially_const.carbon
// CHECK:STDOUT:
// CHECK:STDOUT: constants {
// CHECK:STDOUT: %int_32: Core.IntLiteral = int_value 32 [template]
// CHECK:STDOUT: %i32: type = class_type @Int, @Int(%int_32) [template]
// CHECK:STDOUT: %struct_type.a.b.c.1: type = struct_type {.a: %i32, .b: %i32, .c: %i32} [template]
// CHECK:STDOUT: %Make.type: type = fn_type @Make [template]
// CHECK:STDOUT: %Make: %Make.type = struct_value () [template]
// CHECK:STDOUT: %int_0.1: Core.IntLiteral = int_value 0 [template]
// CHECK:STDOUT: %struct_type.a.b.c.2: type = struct_type {.a: Core.IntLiteral, .b: %i32, .c: Core.IntLiteral} [template]
// CHECK:STDOUT: %Convert.type.2: type = fn_type @Convert.1, @ImplicitAs(%i32) [template]
// CHECK:STDOUT: %Convert.type.10: type = fn_type @Convert.2, @impl.1(%int_32) [template]
// CHECK:STDOUT: %Convert.10: %Convert.type.10 = struct_value () [template]
// CHECK:STDOUT: %interface.5: <witness> = interface_witness (%Convert.10) [template]
// CHECK:STDOUT: %Convert.bound: <bound method> = bound_method %int_0.1, %Convert.10 [template]
// CHECK:STDOUT: %Convert.specific_fn: <specific function> = specific_function %Convert.bound, @Convert.2(%int_32) [template]
// CHECK:STDOUT: %int_0.2: %i32 = int_value 0 [template]
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: imports {
// CHECK:STDOUT: %Core: <namespace> = namespace file.%Core.import, [template] {
// CHECK:STDOUT: .Int = %import_ref.1
// CHECK:STDOUT: .ImplicitAs = %import_ref.5
// CHECK:STDOUT: import Core//prelude
// CHECK:STDOUT: import Core//prelude/...
// CHECK:STDOUT: }
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: file {
// CHECK:STDOUT: package: <namespace> = namespace [template] {
// CHECK:STDOUT: .Core = imports.%Core
// CHECK:STDOUT: .Make = %Make.decl
// CHECK:STDOUT: }
// CHECK:STDOUT: %Core.import = import Core
// CHECK:STDOUT: %Make.decl: %Make.type = fn_decl @Make [template = constants.%Make] {
// CHECK:STDOUT: %n.patt: %i32 = binding_pattern n
// CHECK:STDOUT: %n.param_patt: %i32 = value_param_pattern %n.patt, runtime_param0
// CHECK:STDOUT: %return.patt: %struct_type.a.b.c.1 = return_slot_pattern
// CHECK:STDOUT: %return.param_patt: %struct_type.a.b.c.1 = out_param_pattern %return.patt, runtime_param1
// CHECK:STDOUT: } {
// CHECK:STDOUT: %int_32.loc11_12: Core.IntLiteral = int_value 32 [template = constants.%int_32]
// CHECK:STDOUT: %i32.loc11_12: type = class_type @Int, @Int(constants.%int_32) [template = constants.%i32]
// CHECK:STDOUT: %int_32.loc11_25: Core.IntLiteral = int_value 32 [template = constants.%int_32]
// CHECK:STDOUT: %i32.loc11_25: type = class_type @Int, @Int(constants.%int_32) [template = constants.%i32]
// CHECK:STDOUT: %int_32.loc11_34: Core.IntLiteral = int_value 32 [template = constants.%int_32]
// CHECK:STDOUT: %i32.loc11_34: type = class_type @Int, @Int(constants.%int_32) [template = constants.%i32]
// CHECK:STDOUT: %int_32.loc11_43: Core.IntLiteral = int_value 32 [template = constants.%int_32]
// CHECK:STDOUT: %i32.loc11_43: type = class_type @Int, @Int(constants.%int_32) [template = constants.%i32]
// CHECK:STDOUT: %struct_type.a.b.c: type = struct_type {.a: %i32, .b: %i32, .c: %i32} [template = constants.%struct_type.a.b.c.1]
// CHECK:STDOUT: %n.param: %i32 = value_param runtime_param0
// CHECK:STDOUT: %n: %i32 = bind_name n, %n.param
// CHECK:STDOUT: %return.param: ref %struct_type.a.b.c.1 = out_param runtime_param1
// CHECK:STDOUT: %return: ref %struct_type.a.b.c.1 = return_slot %return.param
// CHECK:STDOUT: }
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: fn @Make(%n.param_patt: %i32) -> %return.param_patt: %struct_type.a.b.c.1 {
// CHECK:STDOUT: !entry:
// CHECK:STDOUT: %int_0.loc12_16: Core.IntLiteral = int_value 0 [template = constants.%int_0.1]
// CHECK:STDOUT: %n.ref: %i32 = name_ref n, %n
// CHECK:STDOUT: %int_0.loc12_32: Core.IntLiteral = int_value 0 [template = constants.%int_0.1]
// CHECK:STDOUT: %.loc12_33.1: %struct_type.a.b.c.2 = struct_literal (%int_0.loc12_16, %n.ref, %int_0.loc12_32)
// CHECK:STDOUT: %impl.elem0.loc12_33.1: %Convert.type.2 = interface_witness_access constants.%interface.5, element0 [template = constants.%Convert.10]
// CHECK:STDOUT: %Convert.bound.loc12_33.1: <bound method> = bound_method %int_0.loc12_16, %impl.elem0.loc12_33.1 [template = constants.%Convert.bound]
// CHECK:STDOUT: %Convert.specific_fn.loc12_33.1: <specific function> = specific_function %Convert.bound.loc12_33.1, @Convert.2(constants.%int_32) [template = constants.%Convert.specific_fn]
// CHECK:STDOUT: %int.convert_checked.loc12_33.1: init %i32 = call %Convert.specific_fn.loc12_33.1(%int_0.loc12_16) [template = constants.%int_0.2]
// CHECK:STDOUT: %.loc12_33.2: init %i32 = converted %int_0.loc12_16, %int.convert_checked.loc12_33.1 [template = constants.%int_0.2]
// CHECK:STDOUT: %.loc12_33.3: ref %i32 = struct_access %return, element0
// CHECK:STDOUT: %.loc12_33.4: init %i32 = initialize_from %.loc12_33.2 to %.loc12_33.3 [template = constants.%int_0.2]
// CHECK:STDOUT: %.loc12_33.5: ref %i32 = struct_access %return, element1
// CHECK:STDOUT: %.loc12_33.6: init %i32 = initialize_from %n.ref to %.loc12_33.5
// CHECK:STDOUT: %impl.elem0.loc12_33.2: %Convert.type.2 = interface_witness_access constants.%interface.5, element0 [template = constants.%Convert.10]
// CHECK:STDOUT: %Convert.bound.loc12_33.2: <bound method> = bound_method %int_0.loc12_32, %impl.elem0.loc12_33.2 [template = constants.%Convert.bound]
// CHECK:STDOUT: %Convert.specific_fn.loc12_33.2: <specific function> = specific_function %Convert.bound.loc12_33.2, @Convert.2(constants.%int_32) [template = constants.%Convert.specific_fn]
// CHECK:STDOUT: %int.convert_checked.loc12_33.2: init %i32 = call %Convert.specific_fn.loc12_33.2(%int_0.loc12_32) [template = constants.%int_0.2]
// CHECK:STDOUT: %.loc12_33.7: init %i32 = converted %int_0.loc12_32, %int.convert_checked.loc12_33.2 [template = constants.%int_0.2]
// CHECK:STDOUT: %.loc12_33.8: ref %i32 = struct_access %return, element2
// CHECK:STDOUT: %.loc12_33.9: init %i32 = initialize_from %.loc12_33.7 to %.loc12_33.8 [template = constants.%int_0.2]
// CHECK:STDOUT: %.loc12_33.10: init %struct_type.a.b.c.1 = struct_init (%.loc12_33.4, %.loc12_33.6, %.loc12_33.9) to %return
// CHECK:STDOUT: %.loc12_34: init %struct_type.a.b.c.1 = converted %.loc12_33.1, %.loc12_33.10
// CHECK:STDOUT: return %.loc12_34 to %return
// CHECK:STDOUT: }
// CHECK:STDOUT:
29 changes: 27 additions & 2 deletions toolchain/lower/handle_aggregates.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,37 @@ static auto EmitAggregateInitializer(FunctionContext& context,
SemIR::InstBlockId refs_id,
llvm::Twine name) -> llvm::Value* {
auto* llvm_type = context.GetType(type_id);
auto refs = context.sem_ir().inst_blocks().Get(refs_id);

switch (SemIR::InitRepr::ForType(context.sem_ir(), type_id).kind) {
case SemIR::InitRepr::None:
case SemIR::InitRepr::InPlace:
case SemIR::InitRepr::None: {
// TODO: Add a helper to poison a value slot.
return llvm::PoisonValue::get(llvm_type);
}

case SemIR::InitRepr::InPlace: {
// Finish initialization of constant fields. We will have skipped this
// when emitting the initializers because they have constant values.
//
// TODO: This emits the initializers for constant fields after all
// initialization of non-constant fields. This may be observable in some
// ways such as under a debugger in a debug build. It would be preferable
// to initialize the constant portions of the aggregate first, but this
// will likely need a change to the SemIR representation.
//
// TODO: If most of the bytes of the result have known constant values,
// it'd be nice to emit a memcpy from a constant followed by the
// non-constant initialization.
for (auto [i, ref_id] : llvm::enumerate(refs)) {
if (context.sem_ir().constant_values().Get(ref_id).is_constant()) {
auto init =
context.sem_ir().insts().GetAs<SemIR::InitializeFrom>(ref_id);
HandleInst(context, ref_id, init);
}
}
// TODO: Add a helper to poison a value slot.
return llvm::PoisonValue::get(llvm_type);
}

case SemIR::InitRepr::ByCopy: {
auto refs = context.sem_ir().inst_blocks().Get(refs_id);
Expand Down
1 change: 1 addition & 0 deletions toolchain/lower/testdata/return/return_var.carbon
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ fn Make() -> C {
// CHECK:STDOUT: %.loc18_30.3.data = getelementptr inbounds nuw { i32, ptr }, ptr %return, i32 0, i32 0, !dbg !7
// CHECK:STDOUT: %.loc18_30.5.next = getelementptr inbounds nuw { i32, ptr }, ptr %return, i32 0, i32 1, !dbg !7
// CHECK:STDOUT: store ptr %return, ptr %.loc18_30.5.next, align 8, !dbg !7
// CHECK:STDOUT: store i32 42, ptr %.loc18_30.3.data, align 4, !dbg !7
// CHECK:STDOUT: ret void, !dbg !8
// CHECK:STDOUT: }
// CHECK:STDOUT:
Expand Down
40 changes: 40 additions & 0 deletions toolchain/lower/testdata/struct/partially_const.carbon
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
// Exceptions. See /LICENSE for license information.
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
// AUTOUPDATE
// TIP: To test this file alone, run:
// TIP: bazel test //toolchain/testing:file_test --test_arg=--file_tests=toolchain/lower/testdata/struct/partially_const.carbon
// TIP: To dump output, run:
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/lower/testdata/struct/partially_const.carbon

fn Make(n: i32) -> {.a: i32, .b: i32, .c: i32} {
return {.a = 0, .b = n, .c = 0};
}

// CHECK:STDOUT: ; ModuleID = 'partially_const.carbon'
// CHECK:STDOUT: source_filename = "partially_const.carbon"
// CHECK:STDOUT:
// CHECK:STDOUT: define void @_CMake.Main(ptr sret({ i32, i32, i32 }) %return, i32 %n) !dbg !4 {
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %.loc12_33.3.a = getelementptr inbounds nuw { i32, i32, i32 }, ptr %return, i32 0, i32 0, !dbg !7
// CHECK:STDOUT: %.loc12_33.5.b = getelementptr inbounds nuw { i32, i32, i32 }, ptr %return, i32 0, i32 1, !dbg !7
// CHECK:STDOUT: store i32 %n, ptr %.loc12_33.5.b, align 4, !dbg !7
// CHECK:STDOUT: %.loc12_33.8.c = getelementptr inbounds nuw { i32, i32, i32 }, ptr %return, i32 0, i32 2, !dbg !7
// CHECK:STDOUT: store i32 0, ptr %.loc12_33.3.a, align 4, !dbg !7
// CHECK:STDOUT: store i32 0, ptr %.loc12_33.8.c, align 4, !dbg !7
// CHECK:STDOUT: ret void, !dbg !8
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: !llvm.module.flags = !{!0, !1}
// CHECK:STDOUT: !llvm.dbg.cu = !{!2}
// CHECK:STDOUT:
// CHECK:STDOUT: !0 = !{i32 7, !"Dwarf Version", i32 5}
// CHECK:STDOUT: !1 = !{i32 2, !"Debug Info Version", i32 3}
// CHECK:STDOUT: !2 = distinct !DICompileUnit(language: DW_LANG_C, file: !3, producer: "carbon", isOptimized: false, runtimeVersion: 0, emissionKind: FullDebug)
// CHECK:STDOUT: !3 = !DIFile(filename: "partially_const.carbon", directory: "")
// CHECK:STDOUT: !4 = distinct !DISubprogram(name: "Make", linkageName: "_CMake.Main", scope: null, file: !3, line: 11, type: !5, spFlags: DISPFlagDefinition, unit: !2)
// CHECK:STDOUT: !5 = !DISubroutineType(types: !6)
// CHECK:STDOUT: !6 = !{}
// CHECK:STDOUT: !7 = !DILocation(line: 12, column: 10, scope: !4)
// CHECK:STDOUT: !8 = !DILocation(line: 12, column: 3, scope: !4)

0 comments on commit b54a27e

Please sign in to comment.