diff --git a/toolchain/check/handle_index.cpp b/toolchain/check/handle_index.cpp index 3de689e5b743..3426a8f45a41 100644 --- a/toolchain/check/handle_index.cpp +++ b/toolchain/check/handle_index.cpp @@ -63,9 +63,24 @@ auto HandleIndexExpression(Context& context, Parse::Node parse_node) -> bool { auto cast_index_id = ConvertToValueOfType( context, index_node.parse_node(), index_node_id, context.CanonicalizeType(SemIR::NodeId::BuiltinIntegerType)); - context.AddNodeAndPush(parse_node, SemIR::Node::ArrayIndex::Make( - parse_node, element_type_id, - operand_node_id, cast_index_id)); + auto array_cat = + SemIR::GetExpressionCategory(context.semantics_ir(), operand_node_id); + if (array_cat == SemIR::ExpressionCategory::Value) { + // If the operand is an array value, convert it to an ephemeral + // reference to an array so we can perform a primitive indexing into it. + operand_node_id = context.AddNode(SemIR::Node::ValueAsReference::Make( + parse_node, operand_type_id, operand_node_id)); + } + auto elem_id = context.AddNode(SemIR::Node::ArrayIndex::Make( + parse_node, element_type_id, operand_node_id, cast_index_id)); + if (array_cat != SemIR::ExpressionCategory::DurableReference) { + // Indexing a durable reference gives a durable reference expression. + // Indexing anything else gives a value expression. + // TODO: This should be replaced by a choice between using `IndexWith` + // and `IndirectIndexWith`. + elem_id = ConvertToValueExpression(context, elem_id); + } + context.node_stack().Push(parse_node, elem_id); return true; } case SemIR::NodeKind::TupleType: { diff --git a/toolchain/check/testdata/array/function_param.carbon b/toolchain/check/testdata/array/function_param.carbon index 7b45de272b63..5b6a848ceb78 100644 --- a/toolchain/check/testdata/array/function_param.carbon +++ b/toolchain/check/testdata/array/function_param.carbon @@ -19,8 +19,10 @@ fn G() -> i32 { // CHECK:STDOUT: // CHECK:STDOUT: fn @F(%arr: [i32; 3], %i: i32) -> i32 { // CHECK:STDOUT: !entry: -// CHECK:STDOUT: %.loc8: i32 = array_index %arr, %i -// CHECK:STDOUT: return %.loc8 +// CHECK:STDOUT: %.loc8_15.1: ref [i32; 3] = value_as_reference %arr +// CHECK:STDOUT: %.loc8_15.2: ref i32 = array_index %.loc8_15.1, %i +// CHECK:STDOUT: %.loc8_15.3: i32 = bind_value %.loc8_15.2 +// CHECK:STDOUT: return %.loc8_15.3 // CHECK:STDOUT: } // CHECK:STDOUT: // CHECK:STDOUT: fn @G() -> i32 { diff --git a/toolchain/check/testdata/index/expression_category.carbon b/toolchain/check/testdata/index/expression_category.carbon new file mode 100644 index 000000000000..20509672eda6 --- /dev/null +++ b/toolchain/check/testdata/index/expression_category.carbon @@ -0,0 +1,102 @@ +// 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 + +fn F() -> [i32; 3]; + +fn G(b: [i32; 3]) { + var a: [i32; 3] = (1, 2, 3); + + // Indexing a durable array reference gives a durable reference. + var pa: i32* = &a[0]; + a[0] = 4; +} + +fn ValueBinding(b: [i32; 3]) { + var a: [i32; 3] = (1, 2, 3); + + // Index but don't do anything else so we can check that a value binding is + // produced when appropriate. + a[0]; + b[0]; + F()[0]; +} + +// CHECK:STDOUT: file "expression_category.carbon" { +// CHECK:STDOUT: %.loc7 = fn_decl @F +// CHECK:STDOUT: %.loc9 = fn_decl @G +// CHECK:STDOUT: %.loc17 = fn_decl @ValueBinding +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @F() -> %return: [i32; 3]; +// CHECK:STDOUT: +// CHECK:STDOUT: fn @G(%b: [i32; 3]) { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: %.loc10_16: i32 = int_literal 3 +// CHECK:STDOUT: %.loc10_17: type = array_type %.loc10_16, i32 +// CHECK:STDOUT: %a: ref [i32; 3] = var "a" +// CHECK:STDOUT: %.loc10_22: i32 = int_literal 1 +// CHECK:STDOUT: %.loc10_25: i32 = int_literal 2 +// CHECK:STDOUT: %.loc10_28: i32 = int_literal 3 +// CHECK:STDOUT: %.loc10_29.1: type = tuple_type (i32, i32, i32) +// CHECK:STDOUT: %.loc10_29.2: (i32, i32, i32) = tuple_literal (%.loc10_22, %.loc10_25, %.loc10_28) +// CHECK:STDOUT: %.loc10_29.3: i32 = int_literal 0 +// CHECK:STDOUT: %.loc10_29.4: ref i32 = array_index %a, %.loc10_29.3 +// CHECK:STDOUT: %.loc10_29.5: init i32 = initialize_from %.loc10_22 to %.loc10_29.4 +// CHECK:STDOUT: %.loc10_29.6: i32 = int_literal 1 +// CHECK:STDOUT: %.loc10_29.7: ref i32 = array_index %a, %.loc10_29.6 +// CHECK:STDOUT: %.loc10_29.8: init i32 = initialize_from %.loc10_25 to %.loc10_29.7 +// CHECK:STDOUT: %.loc10_29.9: i32 = int_literal 2 +// CHECK:STDOUT: %.loc10_29.10: ref i32 = array_index %a, %.loc10_29.9 +// CHECK:STDOUT: %.loc10_29.11: init i32 = initialize_from %.loc10_28 to %.loc10_29.10 +// CHECK:STDOUT: %.loc10_29.12: init [i32; 3] = array_init %.loc10_29.2, (%.loc10_29.5, %.loc10_29.8, %.loc10_29.11) to %a +// CHECK:STDOUT: assign %a, %.loc10_29.12 +// CHECK:STDOUT: %.loc13_14: type = ptr_type i32 +// CHECK:STDOUT: %pa: ref i32* = var "pa" +// CHECK:STDOUT: %.loc13_21: i32 = int_literal 0 +// CHECK:STDOUT: %.loc13_22: ref i32 = array_index %a, %.loc13_21 +// CHECK:STDOUT: %.loc13_18: i32* = address_of %.loc13_22 +// CHECK:STDOUT: assign %pa, %.loc13_18 +// CHECK:STDOUT: %.loc14_5: i32 = int_literal 0 +// CHECK:STDOUT: %.loc14_6: ref i32 = array_index %a, %.loc14_5 +// CHECK:STDOUT: %.loc14_10: i32 = int_literal 4 +// CHECK:STDOUT: assign %.loc14_6, %.loc14_10 +// CHECK:STDOUT: return +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @ValueBinding(%b: [i32; 3]) { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: %.loc18_16: i32 = int_literal 3 +// CHECK:STDOUT: %.loc18_17: type = array_type %.loc18_16, i32 +// CHECK:STDOUT: %a: ref [i32; 3] = var "a" +// CHECK:STDOUT: %.loc18_22: i32 = int_literal 1 +// CHECK:STDOUT: %.loc18_25: i32 = int_literal 2 +// CHECK:STDOUT: %.loc18_28: i32 = int_literal 3 +// CHECK:STDOUT: %.loc18_29.1: (i32, i32, i32) = tuple_literal (%.loc18_22, %.loc18_25, %.loc18_28) +// CHECK:STDOUT: %.loc18_29.2: i32 = int_literal 0 +// CHECK:STDOUT: %.loc18_29.3: ref i32 = array_index %a, %.loc18_29.2 +// CHECK:STDOUT: %.loc18_29.4: init i32 = initialize_from %.loc18_22 to %.loc18_29.3 +// CHECK:STDOUT: %.loc18_29.5: i32 = int_literal 1 +// CHECK:STDOUT: %.loc18_29.6: ref i32 = array_index %a, %.loc18_29.5 +// CHECK:STDOUT: %.loc18_29.7: init i32 = initialize_from %.loc18_25 to %.loc18_29.6 +// CHECK:STDOUT: %.loc18_29.8: i32 = int_literal 2 +// CHECK:STDOUT: %.loc18_29.9: ref i32 = array_index %a, %.loc18_29.8 +// CHECK:STDOUT: %.loc18_29.10: init i32 = initialize_from %.loc18_28 to %.loc18_29.9 +// CHECK:STDOUT: %.loc18_29.11: init [i32; 3] = array_init %.loc18_29.1, (%.loc18_29.4, %.loc18_29.7, %.loc18_29.10) to %a +// CHECK:STDOUT: assign %a, %.loc18_29.11 +// CHECK:STDOUT: %.loc22_5: i32 = int_literal 0 +// CHECK:STDOUT: %.loc22_6: ref i32 = array_index %a, %.loc22_5 +// CHECK:STDOUT: %.loc23_5: i32 = int_literal 0 +// CHECK:STDOUT: %.loc23_6.1: ref [i32; 3] = value_as_reference %b +// CHECK:STDOUT: %.loc23_6.2: ref i32 = array_index %.loc23_6.1, %.loc23_5 +// CHECK:STDOUT: %.loc23_6.3: i32 = bind_value %.loc23_6.2 +// CHECK:STDOUT: %.loc24_4.1: ref [i32; 3] = temporary_storage +// CHECK:STDOUT: %.loc24_4.2: init [i32; 3] = call @F() to %.loc24_4.1 +// CHECK:STDOUT: %.loc24_7: i32 = int_literal 0 +// CHECK:STDOUT: %.loc24_4.3: ref [i32; 3] = temporary %.loc24_4.1, %.loc24_4.2 +// CHECK:STDOUT: %.loc24_8.1: ref i32 = array_index %.loc24_4.3, %.loc24_7 +// CHECK:STDOUT: %.loc24_8.2: i32 = bind_value %.loc24_8.1 +// CHECK:STDOUT: return +// CHECK:STDOUT: } diff --git a/toolchain/check/testdata/index/fail_expression_category.carbon b/toolchain/check/testdata/index/fail_expression_category.carbon new file mode 100644 index 000000000000..199cd3ae3e28 --- /dev/null +++ b/toolchain/check/testdata/index/fail_expression_category.carbon @@ -0,0 +1,74 @@ +// 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 + +fn F() -> [i32; 3]; + +fn G(b: [i32; 3]) { + // Indexing an array value gives a value. + // CHECK:STDERR: fail_expression_category.carbon:[[@LINE+3]]:18: ERROR: Cannot take the address of non-reference expression. + // CHECK:STDERR: var pb: i32* = &b[0]; + // CHECK:STDERR: ^ + var pb: i32* = &b[0]; + // CHECK:STDERR: fail_expression_category.carbon:[[@LINE+3]]:6: ERROR: Expression is not assignable. + // CHECK:STDERR: b[0] = 4; + // CHECK:STDERR: ^ + b[0] = 4; + + // Indexing an ephemeral reference (materialized from an initializing + // expression) gives a value. + // CHECK:STDERR: fail_expression_category.carbon:[[@LINE+3]]:18: ERROR: Cannot take the address of non-reference expression. + // CHECK:STDERR: var pf: i32* = &F()[0]; + // CHECK:STDERR: ^ + var pf: i32* = &F()[0]; + // CHECK:STDERR: fail_expression_category.carbon:[[@LINE+3]]:8: ERROR: Expression is not assignable. + // CHECK:STDERR: F()[0] = 4; + // CHECK:STDERR: ^ + F()[0] = 4; +} + +// CHECK:STDOUT: file "fail_expression_category.carbon" { +// CHECK:STDOUT: %.loc7 = fn_decl @F +// CHECK:STDOUT: %.loc9 = fn_decl @G +// CHECK:STDOUT: } +// CHECK:STDOUT: +// CHECK:STDOUT: fn @F() -> %return: [i32; 3]; +// CHECK:STDOUT: +// CHECK:STDOUT: fn @G(%b: [i32; 3]) { +// CHECK:STDOUT: !entry: +// CHECK:STDOUT: %.loc14_14: type = ptr_type i32 +// CHECK:STDOUT: %pb: ref i32* = var "pb" +// CHECK:STDOUT: %.loc14_21: i32 = int_literal 0 +// CHECK:STDOUT: %.loc14_22.1: ref [i32; 3] = value_as_reference %b +// CHECK:STDOUT: %.loc14_22.2: ref i32 = array_index %.loc14_22.1, %.loc14_21 +// CHECK:STDOUT: %.loc14_22.3: i32 = bind_value %.loc14_22.2 +// CHECK:STDOUT: %.loc14_18: i32* = address_of %.loc14_22.3 +// CHECK:STDOUT: assign %pb, %.loc14_18 +// CHECK:STDOUT: %.loc18_5: i32 = int_literal 0 +// CHECK:STDOUT: %.loc18_6.1: ref [i32; 3] = value_as_reference %b +// CHECK:STDOUT: %.loc18_6.2: ref i32 = array_index %.loc18_6.1, %.loc18_5 +// CHECK:STDOUT: %.loc18_6.3: i32 = bind_value %.loc18_6.2 +// CHECK:STDOUT: %.loc18_10: i32 = int_literal 4 +// CHECK:STDOUT: assign %.loc18_6.3, %.loc18_10 +// CHECK:STDOUT: %.loc25_14: type = ptr_type i32 +// CHECK:STDOUT: %pf: ref i32* = var "pf" +// CHECK:STDOUT: %.loc25_20.1: ref [i32; 3] = temporary_storage +// CHECK:STDOUT: %.loc25_20.2: init [i32; 3] = call @F() to %.loc25_20.1 +// CHECK:STDOUT: %.loc25_23: i32 = int_literal 0 +// CHECK:STDOUT: %.loc25_20.3: ref [i32; 3] = temporary %.loc25_20.1, %.loc25_20.2 +// CHECK:STDOUT: %.loc25_24.1: ref i32 = array_index %.loc25_20.3, %.loc25_23 +// CHECK:STDOUT: %.loc25_24.2: i32 = bind_value %.loc25_24.1 +// CHECK:STDOUT: %.loc25_18: i32* = address_of %.loc25_24.2 +// CHECK:STDOUT: assign %pf, %.loc25_18 +// CHECK:STDOUT: %.loc29_4.1: ref [i32; 3] = temporary_storage +// CHECK:STDOUT: %.loc29_4.2: init [i32; 3] = call @F() to %.loc29_4.1 +// CHECK:STDOUT: %.loc29_7: i32 = int_literal 0 +// CHECK:STDOUT: %.loc29_4.3: ref [i32; 3] = temporary %.loc29_4.1, %.loc29_4.2 +// CHECK:STDOUT: %.loc29_8.1: ref i32 = array_index %.loc29_4.3, %.loc29_7 +// CHECK:STDOUT: %.loc29_8.2: i32 = bind_value %.loc29_8.1 +// CHECK:STDOUT: %.loc29_12: i32 = int_literal 4 +// CHECK:STDOUT: assign %.loc29_8.2, %.loc29_12 +// CHECK:STDOUT: return +// CHECK:STDOUT: } diff --git a/toolchain/lower/function_context.cpp b/toolchain/lower/function_context.cpp index 839361c90acc..320136235010 100644 --- a/toolchain/lower/function_context.cpp +++ b/toolchain/lower/function_context.cpp @@ -98,7 +98,7 @@ auto FunctionContext::CopyValue(SemIR::TypeId type_id, SemIR::NodeId source_id, case SemIR::ValueRepresentation::None: break; case SemIR::ValueRepresentation::Copy: - builder().CreateStore(GetLocalLoaded(source_id), GetLocal(dest_id)); + builder().CreateStore(GetLocal(source_id), GetLocal(dest_id)); break; case SemIR::ValueRepresentation::Pointer: { const auto& layout = llvm_module().getDataLayout(); @@ -118,15 +118,4 @@ auto FunctionContext::CopyValue(SemIR::TypeId type_id, SemIR::NodeId source_id, } } -auto FunctionContext::GetLocalLoaded(SemIR::NodeId node_id) -> llvm::Value* { - auto* value = GetLocal(node_id); - if (llvm::isa(value)) { - auto* load_type = GetType(semantics_ir().GetNode(node_id).type_id()); - return builder().CreateLoad(load_type, value); - } else { - // No load is needed. - return value; - } -} - } // namespace Carbon::Lower diff --git a/toolchain/lower/function_context.h b/toolchain/lower/function_context.h index 9b2df0b2f48f..5bba15aec949 100644 --- a/toolchain/lower/function_context.h +++ b/toolchain/lower/function_context.h @@ -51,26 +51,12 @@ class FunctionContext { return it->second; } - // Returns a local (versus global) value for the given node in loaded state. - // Loads will only be inserted on an as-needed basis. - auto GetLocalLoaded(SemIR::NodeId node_id) -> llvm::Value*; - // Sets the value for the given node. auto SetLocal(SemIR::NodeId node_id, llvm::Value* value) { bool added = locals_.insert({node_id, value}).second; CARBON_CHECK(added) << "Duplicate local insert: " << node_id; } - // Returns the requested index into val based on whether val is a pointer - // type. - auto GetIndexFromStructOrArray(llvm::Type* llvm_type, llvm::Value* val, - unsigned idx, const llvm::Twine& name) - -> llvm::Value* { - return val->getType()->isPointerTy() - ? builder().CreateStructGEP(llvm_type, val, idx, name) - : builder().CreateExtractValue(val, idx, name); - } - // Gets a callable's function. auto GetFunction(SemIR::FunctionId function_id) -> llvm::Function* { return file_context_->GetFunction(function_id); diff --git a/toolchain/lower/handle.cpp b/toolchain/lower/handle.cpp index c93b0458575f..9ef4dc2f6bcc 100644 --- a/toolchain/lower/handle.cpp +++ b/toolchain/lower/handle.cpp @@ -30,24 +30,12 @@ auto HandleArrayIndex(FunctionContext& context, SemIR::NodeId node_id, auto* array_value = context.GetLocal(array_node_id); auto* llvm_type = context.GetType(context.semantics_ir().GetNode(array_node_id).type_id()); - auto index_node = context.semantics_ir().GetNode(index_node_id); - llvm::Value* array_element_value; - - if (index_node.kind() == SemIR::NodeKind::IntegerLiteral) { - const auto index = context.semantics_ir() - .GetIntegerLiteral(index_node.GetAsIntegerLiteral()) - .getZExtValue(); - array_element_value = context.GetIndexFromStructOrArray( - llvm_type, array_value, index, "array.index"); - } else { - auto* index = context.GetLocalLoaded(index_node_id); - // TODO: Handle return value or call such as `F()[a]`. - auto* zero = llvm::ConstantInt::get( - llvm::Type::getInt32Ty(context.llvm_context()), 0); - array_element_value = context.builder().CreateInBoundsGEP( - llvm_type, array_value, {zero, index}, "array.index"); - } - context.SetLocal(node_id, array_element_value); + llvm::Value* indexes[2] = { + llvm::ConstantInt::get(llvm::Type::getInt32Ty(context.llvm_context()), 0), + context.GetLocal(index_node_id)}; + context.SetLocal(node_id, + context.builder().CreateInBoundsGEP(llvm_type, array_value, + indexes, "array.index")); } auto HandleArrayInit(FunctionContext& context, SemIR::NodeId node_id, @@ -108,7 +96,7 @@ auto HandleBranch(FunctionContext& context, SemIR::NodeId /*node_id*/, auto HandleBranchIf(FunctionContext& context, SemIR::NodeId /*node_id*/, SemIR::Node node) -> void { auto [target_block_id, cond_id] = node.GetAsBranchIf(); - llvm::Value* cond = context.GetLocalLoaded(cond_id); + llvm::Value* cond = context.GetLocal(cond_id); llvm::BasicBlock* then_block = context.GetBlock(target_block_id); llvm::BasicBlock* else_block = context.CreateSyntheticBlock(); context.builder().CreateCondBr(cond, then_block, else_block); @@ -118,7 +106,7 @@ auto HandleBranchIf(FunctionContext& context, SemIR::NodeId /*node_id*/, auto HandleBranchWithArg(FunctionContext& context, SemIR::NodeId /*node_id*/, SemIR::Node node) -> void { auto [target_block_id, arg_id] = node.GetAsBranchWithArg(); - llvm::Value* arg = context.GetLocalLoaded(arg_id); + llvm::Value* arg = context.GetLocal(arg_id); SemIR::TypeId arg_type_id = context.semantics_ir().GetNode(arg_id).type_id(); // Opportunistically avoid creating a BasicBlock that contains just a branch. @@ -164,17 +152,9 @@ auto HandleCall(FunctionContext& context, SemIR::NodeId node_id, for (auto ref_id : arg_ids) { auto arg_type_id = context.semantics_ir().GetNode(ref_id).type_id(); - switch (SemIR::GetValueRepresentation(context.semantics_ir(), arg_type_id) - .kind) { - case SemIR::ValueRepresentation::None: - break; - case SemIR::ValueRepresentation::Copy: - case SemIR::ValueRepresentation::Custom: - args.push_back(context.GetLocalLoaded(ref_id)); - break; - case SemIR::ValueRepresentation::Pointer: - args.push_back(context.GetLocal(ref_id)); - break; + if (SemIR::GetValueRepresentation(context.semantics_ir(), arg_type_id) + .kind != SemIR::ValueRepresentation::None) { + args.push_back(context.GetLocal(ref_id)); } } @@ -270,7 +250,7 @@ auto HandleReturnExpression(FunctionContext& context, SemIR::NodeId /*node_id*/, return; case SemIR::InitializingRepresentation::ByCopy: // The expression produces the value representation for the type. - context.builder().CreateRet(context.GetLocalLoaded(expr_id)); + context.builder().CreateRet(context.GetLocal(expr_id)); return; } } @@ -287,11 +267,55 @@ auto HandleStringLiteral(FunctionContext& /*context*/, CARBON_FATAL() << "TODO: Add support: " << node; } +// Extracts an element of either a struct or a tuple by index. Depending on the +// expression category of the aggregate input, this will either produce a value +// or a reference. +static auto GetStructOrTupleElement(FunctionContext& context, + SemIR::NodeId aggr_node_id, unsigned idx, + SemIR::TypeId result_type_id, + llvm::Twine name) -> llvm::Value* { + auto aggr_node = context.semantics_ir().GetNode(aggr_node_id); + auto* aggr_value = context.GetLocal(aggr_node_id); + + auto aggr_cat = + SemIR::GetExpressionCategory(context.semantics_ir(), aggr_node_id); + if (aggr_cat == SemIR::ExpressionCategory::Value && + SemIR::GetValueRepresentation(context.semantics_ir(), aggr_node.type_id()) + .kind == SemIR::ValueRepresentation::Copy) { + // We are holding the values of the aggregate directly, elementwise. + return context.builder().CreateExtractValue(aggr_value, idx, name); + } + + // Either we're accessing an element of a reference and producing a reference, + // or we're accessing an element of a value that is held by pointer and we're + // producing a value. + auto* aggr_type = context.GetType(aggr_node.type_id()); + auto* elem_ptr = + context.builder().CreateStructGEP(aggr_type, aggr_value, idx, name); + + // If this is a value access, load the element if necessary. + if (aggr_cat == SemIR::ExpressionCategory::Value) { + switch ( + SemIR::GetValueRepresentation(context.semantics_ir(), result_type_id) + .kind) { + case SemIR::ValueRepresentation::None: + return llvm::PoisonValue::get(context.GetType(result_type_id)); + case SemIR::ValueRepresentation::Copy: + return context.builder().CreateLoad(context.GetType(result_type_id), + elem_ptr, name + ".load"); + case SemIR::ValueRepresentation::Pointer: + return elem_ptr; + case SemIR::ValueRepresentation::Custom: + CARBON_FATAL() << "TODO: Add support for custom value representation"; + } + } + return elem_ptr; +} + auto HandleStructAccess(FunctionContext& context, SemIR::NodeId node_id, SemIR::Node node) -> void { auto [struct_id, member_index] = node.GetAsStructAccess(); auto struct_type_id = context.semantics_ir().GetNode(struct_id).type_id(); - auto* llvm_type = context.GetType(struct_type_id); // Get type information for member names. auto type_refs = context.semantics_ir().GetNodeBlock( @@ -304,9 +328,9 @@ auto HandleStructAccess(FunctionContext& context, SemIR::NodeId node_id, .GetAsStructTypeField(); auto member_name = context.semantics_ir().GetString(field_name_id); - auto* gep = context.builder().CreateStructGEP( - llvm_type, context.GetLocal(struct_id), member_index.index, member_name); - context.SetLocal(node_id, gep); + context.SetLocal( + node_id, GetStructOrTupleElement(context, struct_id, member_index.index, + node.type_id(), member_name)); } auto HandleStructLiteral(FunctionContext& context, SemIR::NodeId node_id, @@ -400,26 +424,21 @@ auto HandleStructTypeField(FunctionContext& /*context*/, auto HandleTupleAccess(FunctionContext& context, SemIR::NodeId node_id, SemIR::Node node) -> void { auto [tuple_node_id, index] = node.GetAsTupleAccess(); - auto* tuple_value = context.GetLocal(tuple_node_id); - auto* llvm_type = - context.GetType(context.semantics_ir().GetNode(tuple_node_id).type_id()); - context.SetLocal( - node_id, context.GetIndexFromStructOrArray(llvm_type, tuple_value, - index.index, "tuple.elem")); + context.SetLocal(node_id, + GetStructOrTupleElement(context, tuple_node_id, index.index, + node.type_id(), "tuple.elem")); } auto HandleTupleIndex(FunctionContext& context, SemIR::NodeId node_id, SemIR::Node node) -> void { auto [tuple_node_id, index_node_id] = node.GetAsTupleIndex(); - auto* tuple_value = context.GetLocal(tuple_node_id); auto index_node = context.semantics_ir().GetNode(index_node_id); const auto index = context.semantics_ir() .GetIntegerLiteral(index_node.GetAsIntegerLiteral()) .getZExtValue(); - auto* llvm_type = - context.GetType(context.semantics_ir().GetNode(tuple_node_id).type_id()); - context.SetLocal(node_id, context.GetIndexFromStructOrArray( - llvm_type, tuple_value, index, "tuple.index")); + context.SetLocal(node_id, + GetStructOrTupleElement(context, tuple_node_id, index, + node.type_id(), "tuple.index")); } auto HandleTupleLiteral(FunctionContext& context, SemIR::NodeId node_id, diff --git a/toolchain/lower/handle_expression_category.cpp b/toolchain/lower/handle_expression_category.cpp index 3b48becb8e59..8c8faa1c91e6 100644 --- a/toolchain/lower/handle_expression_category.cpp +++ b/toolchain/lower/handle_expression_category.cpp @@ -3,6 +3,7 @@ // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception #include "toolchain/lower/function_context.h" +#include "toolchain/sem_ir/file.h" namespace Carbon::Lower { @@ -45,4 +46,15 @@ auto HandleTemporaryStorage(FunctionContext& context, SemIR::NodeId node_id, nullptr, "temp")); } +auto HandleValueAsReference(FunctionContext& context, SemIR::NodeId node_id, + SemIR::Node node) -> void { + CARBON_CHECK(SemIR::GetExpressionCategory(context.semantics_ir(), + node.GetAsValueAsReference()) == + SemIR::ExpressionCategory::Value); + CARBON_CHECK( + SemIR::GetValueRepresentation(context.semantics_ir(), node.type_id()) + .kind == SemIR::ValueRepresentation::Pointer); + context.SetLocal(node_id, context.GetLocal(node.GetAsValueAsReference())); +} + } // namespace Carbon::Lower diff --git a/toolchain/lower/testdata/function/call/tuple_param_with_return_slot.carbon b/toolchain/lower/testdata/function/call/tuple_param_with_return_slot.carbon index bf90f7c10d9e..d8b8c38cb485 100644 --- a/toolchain/lower/testdata/function/call/tuple_param_with_return_slot.carbon +++ b/toolchain/lower/testdata/function/call/tuple_param_with_return_slot.carbon @@ -18,15 +18,15 @@ fn Main() { // CHECK:STDOUT: define void @F(ptr sret({ i32, i32, i32 }) %return, { i32 } %b, ptr %c) { // CHECK:STDOUT: %tuple.index = extractvalue { i32 } %b, 0 // CHECK:STDOUT: %tuple.index1 = getelementptr inbounds { i32, i32 }, ptr %c, i32 0, i32 0 +// CHECK:STDOUT: %tuple.index.load = load i32, ptr %tuple.index1, align 4 // CHECK:STDOUT: %tuple.index2 = getelementptr inbounds { i32, i32 }, ptr %c, i32 0, i32 1 +// CHECK:STDOUT: %tuple.index.load3 = load i32, ptr %tuple.index2, align 4 // CHECK:STDOUT: %tuple.elem = getelementptr inbounds { i32, i32, i32 }, ptr %return, i32 0, i32 0 // CHECK:STDOUT: store i32 %tuple.index, ptr %tuple.elem, align 4 -// CHECK:STDOUT: %tuple.elem3 = getelementptr inbounds { i32, i32, i32 }, ptr %return, i32 0, i32 1 -// CHECK:STDOUT: %1 = load i32, ptr %tuple.index1, align 4 -// CHECK:STDOUT: store i32 %1, ptr %tuple.elem3, align 4 -// CHECK:STDOUT: %tuple.elem4 = getelementptr inbounds { i32, i32, i32 }, ptr %return, i32 0, i32 2 -// CHECK:STDOUT: %2 = load i32, ptr %tuple.index2, align 4 -// CHECK:STDOUT: store i32 %2, ptr %tuple.elem4, align 4 +// CHECK:STDOUT: %tuple.elem4 = getelementptr inbounds { i32, i32, i32 }, ptr %return, i32 0, i32 1 +// CHECK:STDOUT: store i32 %tuple.index.load, ptr %tuple.elem4, align 4 +// CHECK:STDOUT: %tuple.elem5 = getelementptr inbounds { i32, i32, i32 }, ptr %return, i32 0, i32 2 +// CHECK:STDOUT: store i32 %tuple.index.load3, ptr %tuple.elem5, align 4 // CHECK:STDOUT: ret void // CHECK:STDOUT: } // CHECK:STDOUT: diff --git a/toolchain/lower/testdata/pointer/address_of_field.carbon b/toolchain/lower/testdata/pointer/address_of_field.carbon index bce0c86a58ff..b08d6145532e 100644 --- a/toolchain/lower/testdata/pointer/address_of_field.carbon +++ b/toolchain/lower/testdata/pointer/address_of_field.carbon @@ -8,7 +8,6 @@ fn G(p: i32*); fn F() { var s: {.a: i32, .b: i32} = {.a = 1, .b = 2}; - // TODO: The lowering here is incorrect: we're incorrectly loading `s.b`. G(&s.b); } @@ -24,7 +23,6 @@ fn F() { // CHECK:STDOUT: %b = getelementptr inbounds { i32, i32 }, ptr %s, i32 0, i32 1 // CHECK:STDOUT: store i32 2, ptr %b, align 4 // CHECK:STDOUT: %b1 = getelementptr inbounds { i32, i32 }, ptr %s, i32 0, i32 1 -// CHECK:STDOUT: %1 = load ptr, ptr %b1, align 8 -// CHECK:STDOUT: call void @G(ptr %1) +// CHECK:STDOUT: call void @G(ptr %b1) // CHECK:STDOUT: ret void // CHECK:STDOUT: } diff --git a/toolchain/lower/testdata/pointer/basic.carbon b/toolchain/lower/testdata/pointer/basic.carbon index ae24f7c6c872..82687f1105af 100644 --- a/toolchain/lower/testdata/pointer/basic.carbon +++ b/toolchain/lower/testdata/pointer/basic.carbon @@ -5,7 +5,6 @@ // AUTOUPDATE fn G(p: i32*) -> i32 { - // TODO: The LLVM IR generated here is wrong, missing a load. return *p; } @@ -25,10 +24,9 @@ fn F() -> i32 { // CHECK:STDOUT: define i32 @F() { // CHECK:STDOUT: %n = alloca i32, align 4 // CHECK:STDOUT: store i32 0, ptr %n, align 4 -// CHECK:STDOUT: %1 = load ptr, ptr %n, align 8 -// CHECK:STDOUT: %G = call i32 @G(ptr %1) +// CHECK:STDOUT: %G = call i32 @G(ptr %n) // CHECK:STDOUT: %temp = alloca i32, align 4 // CHECK:STDOUT: store i32 %G, ptr %temp, align 4 -// CHECK:STDOUT: %2 = load i32, ptr %temp, align 4 -// CHECK:STDOUT: ret i32 %2 +// CHECK:STDOUT: %1 = load i32, ptr %temp, align 4 +// CHECK:STDOUT: ret i32 %1 // CHECK:STDOUT: } diff --git a/toolchain/lower/testdata/pointer/pointer_to_pointer.carbon b/toolchain/lower/testdata/pointer/pointer_to_pointer.carbon new file mode 100644 index 000000000000..aa342072eaf5 --- /dev/null +++ b/toolchain/lower/testdata/pointer/pointer_to_pointer.carbon @@ -0,0 +1,29 @@ +// 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 + +fn F(p: i32**) -> i32 { + var a: i32** = p; + var b: i32* = *p; + var c: i32** = &b; + return **c; +} + +// CHECK:STDOUT: ; ModuleID = 'pointer_to_pointer.carbon' +// CHECK:STDOUT: source_filename = "pointer_to_pointer.carbon" +// CHECK:STDOUT: +// CHECK:STDOUT: define i32 @F(ptr %p) { +// CHECK:STDOUT: %a = alloca ptr, align 8 +// CHECK:STDOUT: store ptr %p, ptr %a, align 8 +// CHECK:STDOUT: %b = alloca ptr, align 8 +// CHECK:STDOUT: %1 = load ptr, ptr %p, align 8 +// CHECK:STDOUT: store ptr %1, ptr %b, align 8 +// CHECK:STDOUT: %c = alloca ptr, align 8 +// CHECK:STDOUT: store ptr %b, ptr %c, align 8 +// CHECK:STDOUT: %2 = load ptr, ptr %c, align 8 +// CHECK:STDOUT: %3 = load ptr, ptr %2, align 8 +// CHECK:STDOUT: %4 = load i32, ptr %3, align 4 +// CHECK:STDOUT: ret i32 %4 +// CHECK:STDOUT: } diff --git a/toolchain/sem_ir/file.cpp b/toolchain/sem_ir/file.cpp index f230c2ab45e2..c0d79b921a99 100644 --- a/toolchain/sem_ir/file.cpp +++ b/toolchain/sem_ir/file.cpp @@ -231,6 +231,7 @@ static auto GetTypePrecedence(NodeKind kind) -> int { case NodeKind::TupleInit: case NodeKind::TupleValue: case NodeKind::UnaryOperatorNot: + case NodeKind::ValueAsReference: case NodeKind::VarStorage: CARBON_FATAL() << "GetTypePrecedence for non-type node kind " << kind; } @@ -398,6 +399,7 @@ auto File::StringifyType(TypeId type_id, bool in_type_context) const case NodeKind::TupleInit: case NodeKind::TupleValue: case NodeKind::UnaryOperatorNot: + case NodeKind::ValueAsReference: case NodeKind::VarStorage: // We don't need to handle stringification for nodes that don't show up // in errors, but make it clear what's going on so that it's clearer @@ -518,6 +520,7 @@ auto GetExpressionCategory(const File& file, NodeId node_id) case NodeKind::Temporary: case NodeKind::TemporaryStorage: + case NodeKind::ValueAsReference: return ExpressionCategory::EphemeralReference; } } @@ -569,6 +572,7 @@ auto GetValueRepresentation(const File& file, TypeId type_id) case NodeKind::TupleInit: case NodeKind::TupleValue: case NodeKind::UnaryOperatorNot: + case NodeKind::ValueAsReference: case NodeKind::VarStorage: CARBON_FATAL() << "Type refers to non-type node " << node; diff --git a/toolchain/sem_ir/node.h b/toolchain/sem_ir/node.h index 48fd2984aa7f..749009326333 100644 --- a/toolchain/sem_ir/node.h +++ b/toolchain/sem_ir/node.h @@ -449,6 +449,9 @@ class Node : public Printable { using UnaryOperatorNot = Factory; + using ValueAsReference = + Factory; + using VarStorage = Factory; explicit Node() diff --git a/toolchain/sem_ir/node_kind.def b/toolchain/sem_ir/node_kind.def index 4dda72a6c06d..779036893009 100644 --- a/toolchain/sem_ir/node_kind.def +++ b/toolchain/sem_ir/node_kind.def @@ -103,6 +103,8 @@ CARBON_SEMANTICS_NODE_KIND_IMPL(TupleLiteral, "tuple_literal", Typed, CARBON_SEMANTICS_NODE_KIND_IMPL(TupleType, "tuple_type", Typed, NotTerminator) CARBON_SEMANTICS_NODE_KIND_IMPL(TupleValue, "tuple_value", Typed, NotTerminator) CARBON_SEMANTICS_NODE_KIND_IMPL(UnaryOperatorNot, "not", Typed, NotTerminator) +CARBON_SEMANTICS_NODE_KIND_IMPL(ValueAsReference, "value_as_reference", Typed, + NotTerminator) CARBON_SEMANTICS_NODE_KIND_IMPL(VarStorage, "var", Typed, NotTerminator) #undef CARBON_SEMANTICS_NODE_KIND