Skip to content
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

Fix lowering of a conversion from a type with a pointer value representation to a type with a copy value representation. #4467

Merged
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
8 changes: 4 additions & 4 deletions toolchain/check/convert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -975,8 +975,7 @@ auto Convert(Context& context, SemIR::LocId loc_id, SemIR::InstId expr_id,
}

// If this is not a builtin conversion, try an `ImplicitAs` conversion.
SemIR::Inst expr = sem_ir.insts().Get(expr_id);
if (expr.type_id() != target.type_id) {
if (sem_ir.insts().Get(expr_id).type_id() != target.type_id) {
SemIR::InstId interface_args[] = {
context.types().GetInstId(target.type_id)};
Operator op = {
Expand Down Expand Up @@ -1019,7 +1018,8 @@ auto Convert(Context& context, SemIR::LocId loc_id, SemIR::InstId expr_id,
switch (SemIR::GetExprCategory(sem_ir, expr_id)) {
case SemIR::ExprCategory::NotExpr:
case SemIR::ExprCategory::Mixed:
CARBON_FATAL("Unexpected expression {0} after builtin conversions", expr);
CARBON_FATAL("Unexpected expression {0} after builtin conversions",
sem_ir.insts().Get(expr_id));

case SemIR::ExprCategory::Error:
return SemIR::InstId::BuiltinError;
Expand Down Expand Up @@ -1057,7 +1057,7 @@ auto Convert(Context& context, SemIR::LocId loc_id, SemIR::InstId expr_id,
// TODO: Support types with custom value representations.
expr_id = context.AddInst<SemIR::BindValue>(
context.insts().GetLocId(expr_id),
{.type_id = expr.type_id(), .value_id = expr_id});
{.type_id = target.type_id, .value_id = expr_id});
// We now have a value expression.
[[fallthrough]];

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ fn Test() {
// CHECK:STDOUT: %.loc30_18.7: init i32 = converted %Source.call.loc30, %Convert.call.loc30
// CHECK:STDOUT: %.loc30_18.8: ref i32 = temporary_storage
// CHECK:STDOUT: %.loc30_18.9: ref i32 = temporary %.loc30_18.8, %.loc30_18.7
// CHECK:STDOUT: %.loc30_18.10: %X = bind_value %.loc30_18.9
// CHECK:STDOUT: %.loc30_18.10: i32 = bind_value %.loc30_18.9
// CHECK:STDOUT: %Sink_i32.call: init %.1 = call %Sink_i32.ref(%.loc30_18.10)
// CHECK:STDOUT: %Sink_X.ref: %Sink_X.type = name_ref Sink_X, file.%Sink_X.decl [template = constants.%Sink_X]
// CHECK:STDOUT: %Source.ref.loc31: %Source.type = name_ref Source, file.%Source.decl [template = constants.%Source]
Expand All @@ -334,7 +334,7 @@ fn Test() {
// CHECK:STDOUT: %Convert.call.loc31: init %X = call %.loc31_16.7(%.loc31_16.9) to %.loc31_16.8
// CHECK:STDOUT: %.loc31_16.10: init %X = converted %Source.call.loc31, %Convert.call.loc31
// CHECK:STDOUT: %.loc31_16.11: ref %X = temporary %.loc31_16.8, %.loc31_16.10
// CHECK:STDOUT: %.loc31_16.12: i32 = bind_value %.loc31_16.11
// CHECK:STDOUT: %.loc31_16.12: %X = bind_value %.loc31_16.11
// CHECK:STDOUT: %Sink_X.call: init %.1 = call %Sink_X.ref(%.loc31_16.12)
// CHECK:STDOUT: return
// CHECK:STDOUT: }
Expand Down
6 changes: 6 additions & 0 deletions toolchain/lower/constant.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -260,6 +260,12 @@ auto LowerConstants(FileContext& file_context,
}

auto inst = file_context.sem_ir().insts().Get(inst_id);
if (inst.type_id().is_valid() &&
!file_context.sem_ir().types().IsComplete(inst.type_id())) {
// If a constant doesn't have a complete type, that means we imported it
// but didn't actually use it.
continue;
}
llvm::Constant* value = nullptr;
CARBON_KIND_SWITCH(inst) {
#define CARBON_SEM_IR_INST_KIND(Name) \
Expand Down
3 changes: 2 additions & 1 deletion toolchain/lower/file_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ class FileContext {
auto GetType(SemIR::TypeId type_id) -> llvm::Type* {
// InvalidType should not be passed in.
CARBON_CHECK(type_id.index >= 0, "{0}", type_id);
CARBON_CHECK(types_[type_id.index], "Missing type {0}", type_id);
CARBON_CHECK(types_[type_id.index], "Missing type {0}: {1}", type_id,
sem_ir().types().GetAsInst(type_id));
return types_[type_id.index];
}

Expand Down
81 changes: 81 additions & 0 deletions toolchain/lower/testdata/class/convert.carbon
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// 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/class/convert.carbon
// TIP: To dump output, run:
// TIP: bazel run //toolchain/testing:file_test -- --dump_output --file_tests=toolchain/lower/testdata/class/convert.carbon

class IntWrapper {
var n: i32;
}

impl IntWrapper as Core.ImplicitAs(i32) {
fn Convert[self: Self]() -> i32 { return self.n; }
}

fn Consume(n: i32) {}

fn DoIt() {
var w: IntWrapper = {.n = 42};
Consume(w);
}

// CHECK:STDOUT: ; ModuleID = 'convert.carbon'
// CHECK:STDOUT: source_filename = "convert.carbon"
// CHECK:STDOUT:
// CHECK:STDOUT: @struct.loc22_32 = internal constant { i32 } { i32 42 }
// CHECK:STDOUT:
// CHECK:STDOUT: define i32 @"_CConvert.IntWrapper.Main:ImplicitAs.Core"(ptr %self) !dbg !4 {
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %.loc16_48.1.n = getelementptr inbounds nuw { i32 }, ptr %self, i32 0, i32 0, !dbg !7
// CHECK:STDOUT: %.loc16_48.2 = load i32, ptr %.loc16_48.1.n, align 4, !dbg !7
// CHECK:STDOUT: ret i32 %.loc16_48.2, !dbg !8
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define void @_CConsume.Main(i32 %n) !dbg !9 {
// CHECK:STDOUT: entry:
// CHECK:STDOUT: ret void, !dbg !10
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define void @_CDoIt.Main() !dbg !11 {
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %w.var = alloca { i32 }, align 8, !dbg !12
// CHECK:STDOUT: %.loc22_31.2.n = getelementptr inbounds nuw { i32 }, ptr %w.var, i32 0, i32 0, !dbg !13
// CHECK:STDOUT: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %w.var, ptr align 4 @struct.loc22_32, i64 4, i1 false), !dbg !14
// CHECK:STDOUT: %Convert.call = call i32 @"_CConvert.IntWrapper.Main:ImplicitAs.Core"(ptr %w.var), !dbg !15
// CHECK:STDOUT: %.loc23_11.6.temp = alloca i32, align 4, !dbg !15
// CHECK:STDOUT: store i32 %Convert.call, ptr %.loc23_11.6.temp, align 4, !dbg !15
// CHECK:STDOUT: %.loc23_11.8 = load i32, ptr %.loc23_11.6.temp, align 4, !dbg !15
// CHECK:STDOUT: call void @_CConsume.Main(i32 %.loc23_11.8), !dbg !16
// CHECK:STDOUT: ret void, !dbg !17
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: ; Function Attrs: nocallback nofree nounwind willreturn memory(argmem: readwrite)
// CHECK:STDOUT: declare void @llvm.memcpy.p0.p0.i64(ptr noalias nocapture writeonly, ptr noalias nocapture readonly, i64, i1 immarg) #0
// CHECK:STDOUT:
// CHECK:STDOUT: attributes #0 = { nocallback nofree nounwind willreturn memory(argmem: readwrite) }
// 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: "convert.carbon", directory: "")
// CHECK:STDOUT: !4 = distinct !DISubprogram(name: "Convert", linkageName: "_CConvert.IntWrapper.Main:ImplicitAs.Core", scope: null, file: !3, line: 16, type: !5, spFlags: DISPFlagDefinition, unit: !2)
// CHECK:STDOUT: !5 = !DISubroutineType(types: !6)
// CHECK:STDOUT: !6 = !{}
// CHECK:STDOUT: !7 = !DILocation(line: 16, column: 44, scope: !4)
// CHECK:STDOUT: !8 = !DILocation(line: 16, column: 37, scope: !4)
// CHECK:STDOUT: !9 = distinct !DISubprogram(name: "Consume", linkageName: "_CConsume.Main", scope: null, file: !3, line: 19, type: !5, spFlags: DISPFlagDefinition, unit: !2)
// CHECK:STDOUT: !10 = !DILocation(line: 19, column: 1, scope: !9)
// CHECK:STDOUT: !11 = distinct !DISubprogram(name: "DoIt", linkageName: "_CDoIt.Main", scope: null, file: !3, line: 21, type: !5, spFlags: DISPFlagDefinition, unit: !2)
// CHECK:STDOUT: !12 = !DILocation(line: 22, column: 7, scope: !11)
// CHECK:STDOUT: !13 = !DILocation(line: 22, column: 23, scope: !11)
// CHECK:STDOUT: !14 = !DILocation(line: 22, column: 3, scope: !11)
// CHECK:STDOUT: !15 = !DILocation(line: 23, column: 11, scope: !11)
// CHECK:STDOUT: !16 = !DILocation(line: 23, column: 3, scope: !11)
// CHECK:STDOUT: !17 = !DILocation(line: 21, column: 1, scope: !11)
Loading