Skip to content

Commit

Permalink
Respond to reviewer comments
Browse files Browse the repository at this point in the history
  • Loading branch information
geoffromer committed Oct 31, 2024
1 parent 43d12ae commit 8073bcb
Show file tree
Hide file tree
Showing 109 changed files with 267 additions and 665 deletions.
42 changes: 31 additions & 11 deletions toolchain/lower/file_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -397,25 +397,45 @@ auto FileContext::BuildFunctionDefinition(SemIR::FunctionId function_id)
.GetAs<SemIR::FunctionDecl>(function.latest_decl_id())
.decl_block_id;
}
llvm::ArrayRef<SemIR::InstBlockId> singleton_decl_block = decl_block_id;

// Lower all blocks.
for (auto block_id : llvm::concat<const SemIR::InstBlockId>(
singleton_decl_block, body_block_ids)) {
// Lowers the contents of block_id into the corresponding LLVM block,
// creating it if it doesn't already exist.
auto lower_block = [&](SemIR::InstBlockId block_id) {
CARBON_VLOG("Lowering {0}\n", block_id);
auto* llvm_block = function_lowering.GetBlock(block_id);
// Keep the LLVM blocks in lexical order.
llvm_block->moveBefore(llvm_function->end());
function_lowering.builder().SetInsertPoint(llvm_block);
function_lowering.LowerBlock(block_id);
function_lowering.LowerBlockContents(block_id);
return llvm_block;
};

auto* llvm_decl_block = lower_block(decl_block_id);

// If the decl block is empty, reuse it as the first body block. We don't do
// this when the decl block is non-empty so that any branches back to the
// first body block don't also re-execute the decl.
if (llvm_decl_block->empty()) {
CARBON_CHECK(function_lowering.TryToReuseBlock(body_block_ids.front(),
llvm_decl_block));
} else {
function_lowering.builder().SetInsertPoint(llvm_decl_block);
function_lowering.builder().CreateBr(
function_lowering.GetBlock(body_block_ids.front()));
}

// Add a terminator to the decl block.
function_lowering.builder().SetInsertPoint(
function_lowering.GetBlock(decl_block_id));
function_lowering.builder().CreateBr(
function_lowering.GetBlock(body_block_ids.front()));
function_lowering.builder().ClearInsertionPoint();
// Lower all blocks.
for (auto block_id : body_block_ids) {
lower_block(block_id);
}

// LLVM requires that the entry block has no predecessors.
auto* entry_block = &llvm_function->getEntryBlock();
if (entry_block->hasNPredecessorsOrMore(1)) {
auto* new_entry_block = llvm::BasicBlock::Create(
llvm_context(), "entry", llvm_function, entry_block);
llvm::BranchInst::Create(entry_block, new_entry_block);
}
}

auto FileContext::BuildDISubprogram(const SemIR::Function& function,
Expand Down
2 changes: 1 addition & 1 deletion toolchain/lower/function_context.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ auto FunctionContext::TryToReuseBlock(SemIR::InstBlockId block_id,
return true;
}

auto FunctionContext::LowerBlock(SemIR::InstBlockId block_id) -> void {
auto FunctionContext::LowerBlockContents(SemIR::InstBlockId block_id) -> void {
for (auto inst_id : sem_ir().inst_blocks().Get(block_id)) {
LowerInst(inst_id);
}
Expand Down
2 changes: 1 addition & 1 deletion toolchain/lower/function_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class FunctionContext {
-> bool;

// Builds LLVM IR for the sequence of instructions in `block_id`.
auto LowerBlock(SemIR::InstBlockId block_id) -> void;
auto LowerBlockContents(SemIR::InstBlockId block_id) -> void;

// Builds LLVM IR for the specified instruction.
auto LowerInst(SemIR::InstId inst_id) -> void;
Expand Down
2 changes: 1 addition & 1 deletion toolchain/lower/handle.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ auto HandleInst(FunctionContext& context, SemIR::InstId /*inst_id*/,

auto HandleInst(FunctionContext& context, SemIR::InstId inst_id,
SemIR::SpliceBlock inst) -> void {
context.LowerBlock(inst.block_id);
context.LowerBlockContents(inst.block_id);
context.SetLocal(inst_id, context.GetValue(inst.result_id));
}

Expand Down
4 changes: 1 addition & 3 deletions toolchain/lower/testdata/alias/local.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ fn F() -> i32 {
// CHECK:STDOUT: source_filename = "local.carbon"
// CHECK:STDOUT:
// CHECK:STDOUT: define i32 @_CF.Main() !dbg !4 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %a.var = alloca i32, align 4, !dbg !7
// CHECK:STDOUT: store i32 0, ptr %a.var, align 4, !dbg !8
// CHECK:STDOUT: %.loc14 = load i32, ptr %a.var, align 4, !dbg !9
Expand Down
4 changes: 1 addition & 3 deletions toolchain/lower/testdata/array/array_in_place.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,7 @@ fn G() {
// CHECK:STDOUT: declare void @_CF.Main(ptr sret({ i32, i32, i32 }))
// CHECK:STDOUT:
// CHECK:STDOUT: define void @_CG.Main() !dbg !4 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %v.var = alloca [2 x { i32, i32, i32 }], align 8, !dbg !7
// CHECK:STDOUT: %.loc14_42.2.array.index = getelementptr inbounds [2 x { i32, i32, i32 }], ptr %v.var, i32 0, i32 0, !dbg !8
// CHECK:STDOUT: call void @_CF.Main(ptr %.loc14_42.2.array.index), !dbg !9
Expand Down
8 changes: 2 additions & 6 deletions toolchain/lower/testdata/array/assign_return_value.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -20,19 +20,15 @@ fn Run() {
// CHECK:STDOUT: @tuple.loc11_39 = internal constant { i32, i32 } { i32 12, i32 24 }
// CHECK:STDOUT:
// CHECK:STDOUT: define void @_CF.Main(ptr sret({ i32, i32 }) %return) !dbg !4 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %.loc11_38.2.tuple.elem = getelementptr inbounds nuw { i32, i32 }, ptr %return, i32 0, i32 0, !dbg !7
// CHECK:STDOUT: %.loc11_38.4.tuple.elem = getelementptr inbounds nuw { i32, i32 }, ptr %return, i32 0, i32 1, !dbg !7
// CHECK:STDOUT: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %return, ptr align 4 @tuple.loc11_39, i64 8, i1 false), !dbg !8
// CHECK:STDOUT: ret void, !dbg !8
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define void @main() !dbg !9 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %t.var = alloca [2 x i32], align 4, !dbg !10
// CHECK:STDOUT: %.loc14_22.1.temp = alloca { i32, i32 }, align 8, !dbg !11
// CHECK:STDOUT: call void @_CF.Main(ptr %.loc14_22.1.temp), !dbg !11
Expand Down
4 changes: 1 addition & 3 deletions toolchain/lower/testdata/array/base.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,7 @@ fn Run() {
// CHECK:STDOUT: @tuple.2.loc15_37 = internal constant { i32, i32, i32 } { i32 1, i32 2, i32 3 }
// CHECK:STDOUT:
// CHECK:STDOUT: define void @main() !dbg !4 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %a.var = alloca [1 x i32], align 4, !dbg !7
// CHECK:STDOUT: %.loc12_24.3.array.index = getelementptr inbounds [1 x i32], ptr %a.var, i32 0, i32 0, !dbg !8
// CHECK:STDOUT: call void @llvm.memcpy.p0.p0.i64(ptr align 4 %a.var, ptr align 4 @array.1.loc12_25, i64 4, i1 false), !dbg !9
Expand Down
8 changes: 2 additions & 6 deletions toolchain/lower/testdata/array/function_param.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -22,18 +22,14 @@ fn G() -> i32 {
// CHECK:STDOUT: @array.loc16_20.13 = internal constant [3 x i32] [i32 1, i32 2, i32 3]
// CHECK:STDOUT:
// CHECK:STDOUT: define i32 @_CF.Main(ptr %arr, i32 %i) !dbg !4 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %.loc12_15.2.array.index = getelementptr inbounds [3 x i32], ptr %arr, i32 0, i32 %i, !dbg !7
// CHECK:STDOUT: %.loc12_15.3 = load i32, ptr %.loc12_15.2.array.index, align 4, !dbg !7
// CHECK:STDOUT: ret i32 %.loc12_15.3, !dbg !8
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define i32 @_CG.Main() !dbg !9 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %.loc16_20.2.temp = alloca [3 x i32], align 4, !dbg !10
// CHECK:STDOUT: %.loc16_20.4.array.index = getelementptr inbounds [3 x i32], ptr %.loc16_20.2.temp, i32 0, i32 0, !dbg !10
// CHECK:STDOUT: %.loc16_20.7.array.index = getelementptr inbounds [3 x i32], ptr %.loc16_20.2.temp, i32 0, i32 1, !dbg !10
Expand Down
8 changes: 2 additions & 6 deletions toolchain/lower/testdata/basics/false_true.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,12 @@ fn T() -> bool {
// CHECK:STDOUT: source_filename = "false_true.carbon"
// CHECK:STDOUT:
// CHECK:STDOUT: define i1 @_CF.Main() !dbg !4 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: ret i1 false, !dbg !7
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define i1 @_CT.Main() !dbg !8 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: ret i1 true, !dbg !9
// CHECK:STDOUT: }
// CHECK:STDOUT:
Expand Down
16 changes: 4 additions & 12 deletions toolchain/lower/testdata/basics/int_types.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -17,30 +17,22 @@ fn F_u65536(a: u65536) -> u65536 { return a; }
// CHECK:STDOUT: source_filename = "int_types.carbon"
// CHECK:STDOUT:
// CHECK:STDOUT: define i8 @_CF_i8.Main(i8 %a) !dbg !4 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: ret i8 %a, !dbg !7
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define i16 @_CF_u16.Main(i16 %a) !dbg !8 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: ret i16 %a, !dbg !9
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define i64 @_CF_i64.Main(i64 %a) !dbg !10 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: ret i64 %a, !dbg !11
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define i65536 @_CF_u65536.Main(i65536 %a) !dbg !12 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: ret i65536 %a, !dbg !13
// CHECK:STDOUT: }
// CHECK:STDOUT:
Expand Down
4 changes: 1 addition & 3 deletions toolchain/lower/testdata/basics/numeric_literals.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,7 @@ fn F() {
// CHECK:STDOUT: @array.2.loc27_4 = internal constant [6 x double] [double 9.000000e-01, double 8.000000e+00, double 8.000000e+01, double 1.000000e+07, double 1.000000e+08, double 1.000000e-08]
// CHECK:STDOUT:
// CHECK:STDOUT: define void @_CF.Main() !dbg !4 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %ints.var = alloca [4 x i32], align 4, !dbg !7
// CHECK:STDOUT: %.loc19_3.3.array.index = getelementptr inbounds [4 x i32], ptr %ints.var, i32 0, i32 0, !dbg !8
// CHECK:STDOUT: %.loc19_3.6.array.index = getelementptr inbounds [4 x i32], ptr %ints.var, i32 0, i32 1, !dbg !8
Expand Down
12 changes: 3 additions & 9 deletions toolchain/lower/testdata/basics/type_values.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -29,23 +29,17 @@ fn F64() -> type {
// CHECK:STDOUT: %type = type {}
// CHECK:STDOUT:
// CHECK:STDOUT: define %type @_CI32.Main() !dbg !4 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: ret %type zeroinitializer, !dbg !7
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define %type @_CI48.Main() !dbg !8 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: ret %type zeroinitializer, !dbg !9
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define %type @_CF64.Main() !dbg !10 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: ret %type zeroinitializer, !dbg !11
// CHECK:STDOUT: }
// CHECK:STDOUT:
Expand Down
4 changes: 1 addition & 3 deletions toolchain/lower/testdata/basics/zero.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@ fn Main() -> i32 {
// CHECK:STDOUT: source_filename = "zero.carbon"
// CHECK:STDOUT:
// CHECK:STDOUT: define i32 @_CMain.Main() !dbg !4 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: ret i32 0, !dbg !7
// CHECK:STDOUT: }
// CHECK:STDOUT:
Expand Down
4 changes: 1 addition & 3 deletions toolchain/lower/testdata/builtins/big_int.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ fn Copy(x: Make()) -> Make() {
// CHECK:STDOUT: source_filename = "big_int.carbon"
// CHECK:STDOUT:
// CHECK:STDOUT: define {} @_CCopy.Main({} %x) !dbg !4 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: ret {} %x, !dbg !7
// CHECK:STDOUT: }
// CHECK:STDOUT:
Expand Down
44 changes: 11 additions & 33 deletions toolchain/lower/testdata/builtins/float.carbon
Original file line number Diff line number Diff line change
Expand Up @@ -45,89 +45,67 @@ fn TestGreaterEq(a: f64, b: f64) -> bool { return GreaterEq(a, b); }
// CHECK:STDOUT: source_filename = "float.carbon"
// CHECK:STDOUT:
// CHECK:STDOUT: define double @_CTestNegate.Main(double %a) !dbg !4 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %float.negate = fneg double %a, !dbg !7
// CHECK:STDOUT: ret double %float.negate, !dbg !8
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define double @_CTestAdd.Main(double %a, double %b) !dbg !9 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %float.add = fadd double %a, %b, !dbg !10
// CHECK:STDOUT: ret double %float.add, !dbg !11
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define double @_CTestSub.Main(double %a, double %b) !dbg !12 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %float.sub = fsub double %a, %b, !dbg !13
// CHECK:STDOUT: ret double %float.sub, !dbg !14
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define double @_CTestMul.Main(double %a, double %b) !dbg !15 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %float.mul = fmul double %a, %b, !dbg !16
// CHECK:STDOUT: ret double %float.mul, !dbg !17
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define double @_CTestDiv.Main(double %a, double %b) !dbg !18 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %float.div = fdiv double %a, %b, !dbg !19
// CHECK:STDOUT: ret double %float.div, !dbg !20
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define i1 @_CTestEq.Main(double %a, double %b) !dbg !21 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %float.eq = fcmp oeq double %a, %b, !dbg !22
// CHECK:STDOUT: ret i1 %float.eq, !dbg !23
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define i1 @_CTestNeq.Main(double %a, double %b) !dbg !24 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %float.neq = fcmp one double %a, %b, !dbg !25
// CHECK:STDOUT: ret i1 %float.neq, !dbg !26
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define i1 @_CTestLess.Main(double %a, double %b) !dbg !27 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %float.less = fcmp olt double %a, %b, !dbg !28
// CHECK:STDOUT: ret i1 %float.less, !dbg !29
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define i1 @_CTestLessEq.Main(double %a, double %b) !dbg !30 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %float.less_eq = fcmp ole double %a, %b, !dbg !31
// CHECK:STDOUT: ret i1 %float.less_eq, !dbg !32
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define i1 @_CTestGreater.Main(double %a, double %b) !dbg !33 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %float.greater = fcmp ogt double %a, %b, !dbg !34
// CHECK:STDOUT: ret i1 %float.greater, !dbg !35
// CHECK:STDOUT: }
// CHECK:STDOUT:
// CHECK:STDOUT: define i1 @_CTestGreaterEq.Main(double %a, double %b) !dbg !36 {
// CHECK:STDOUT: br label %entry
// CHECK:STDOUT:
// CHECK:STDOUT: entry: ; preds = %0
// CHECK:STDOUT: entry:
// CHECK:STDOUT: %float.greater_eq = fcmp oge double %a, %b, !dbg !37
// CHECK:STDOUT: ret i1 %float.greater_eq, !dbg !38
// CHECK:STDOUT: }
Expand Down
Loading

0 comments on commit 8073bcb

Please sign in to comment.