Model return slot as parameter in lowering

[geoffromer]

No description provided.

[zygoloid]

I think there's a multiple-ownership issue here: splice_block owns its block, but the decl block is already owned by the function_decl instruction. In practice this is resulting in the decl block being printed twice in SemIR.

Given that different function_decls for the same function will have different decl blocks, I don't think we want to just transfer ownership from the function_decl to the Function entity. I think either of these options could be reasonable:

• Instead of splice_block, use a different instruction that invokes another block without splicing it. We'd need some way of naming the block owned by the function_decl -- maybe the instruction could name the function_decl instruction itself?
• Omit the splice_block entirely and instead make lowering first lower the decl block of the definition and then the function body.

I don't think I have a strong preference between these, but the latter option would avoid the need to add a new kind of special-purpose instruction.

[geoffromer]

• Omit the splice_block entirely and instead make lowering first lower the decl block of the definition and then the function body.

I initially tried something like that, but got stuck on how to give the lowered decl block a suitable terminator. I just tried again, and I think I found a way to make it work without too much mess.

[zygoloid]

Maybe

Suggested change

	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()));
	// 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.
	llvm::BasicBlock* block = context.builder().GetInsertBlock();
	if (block->empty() &&
	function_lowering.TryToReuseBlock(body_block_ids.front(),
	block))) {
	// Reuse this block as the first block of the function body.
	} else {
	function_lowering.builder().CreateBr(
	function_lowering.GetBlock(body_block_ids.front()));

to make this more similar to the other uses of TryToReuseBlock and handle the possibility that we somehow generate multiple LLVM basic blocks while lowering the decl block. (We shouldn't need to SetInsertionPoint because it should still be in the llvm_decl_block.) I wonder if we should actually factor out a function to do this block-reuse or branch process -- we seem to do the same thing three times now.

[geoffromer]

Done, with some tweaks. I assume removing the first lower_block call was an accident?

[zygoloid]

Yes, oops, I just meant to remove the variable not the initializer.