From 806166db9db83bee816d169a13ab9992de9d66a0 Mon Sep 17 00:00:00 2001 From: Philip Herron Date: Tue, 7 Jan 2025 18:15:37 +0000 Subject: [PATCH] gccrs: cleanup our enum type layout to be closer to rustc This changes our enum type layout so for example: enum Foo { A, B, C(char), D { x: i32, y: i32 }, } Used to get layed out like this in gccrs: union { struct A { int RUST$ENUM$DISR; }; struct B { int RUST$ENUM$DISR; }; struct C { int RUST$ENUM$DISR; char __0; }; struct D { int RUST$ENUM$DISR; i64 x; i64 y; }; } This has some issues notably with the constexpr because this is just a giant union it means its not simple to constify what enum variant we are looking at because the discriminant is a mess. This now gets layed out as: struct { int RUST$ENUM$DISR; union { struct A { }; struct B { }; struct C { char __0; }; struct D { i64 x; i64 y; }; } payload; } This layout is much cleaner and allows for our constexpr to work properly. gcc/rust/ChangeLog: * backend/rust-compile-expr.cc (CompileExpr::visit): new layout * backend/rust-compile-pattern.cc (CompilePatternCheckExpr::visit): likewise (CompilePatternBindings::visit): likewise * backend/rust-compile-resolve-path.cc: likewise * backend/rust-compile-type.cc (TyTyResolveCompile::visit): implement new layout * rust-gcc.cc (constructor_expression): get rid of useless assert Signed-off-by: Philip Herron --- gcc/rust/backend/rust-compile-expr.cc | 74 +++++++++++-------- gcc/rust/backend/rust-compile-pattern.cc | 64 ++++++++-------- gcc/rust/backend/rust-compile-resolve-path.cc | 5 +- gcc/rust/backend/rust-compile-type.cc | 62 ++++++++++++---- gcc/rust/rust-gcc.cc | 2 +- 5 files changed, 128 insertions(+), 79 deletions(-) diff --git a/gcc/rust/backend/rust-compile-expr.cc b/gcc/rust/backend/rust-compile-expr.cc index 46b9b0adccfd..6a0db31efe8a 100644 --- a/gcc/rust/backend/rust-compile-expr.cc +++ b/gcc/rust/backend/rust-compile-expr.cc @@ -558,24 +558,32 @@ CompileExpr::visit (HIR::StructExprStructFields &struct_expr) } } - // the constructor depends on whether this is actually an enum or not if - // its an enum we need to setup the discriminator - std::vector ctor_arguments; - if (adt->is_enum ()) + if (!adt->is_enum ()) { - HIR::Expr &discrim_expr = variant->get_discriminant (); - tree discrim_expr_node = CompileExpr::Compile (discrim_expr, ctx); - tree folded_discrim_expr = fold_expr (discrim_expr_node); - tree qualifier = folded_discrim_expr; - - ctor_arguments.push_back (qualifier); + translated + = Backend::constructor_expression (compiled_adt_type, adt->is_enum (), + arguments, union_disriminator, + struct_expr.get_locus ()); + return; } - for (auto &arg : arguments) - ctor_arguments.push_back (arg); + + HIR::Expr &discrim_expr = variant->get_discriminant (); + tree discrim_expr_node = CompileExpr::Compile (discrim_expr, ctx); + tree folded_discrim_expr = fold_expr (discrim_expr_node); + tree qualifier = folded_discrim_expr; + + tree enum_root_files = TYPE_FIELDS (compiled_adt_type); + tree payload_root = DECL_CHAIN (enum_root_files); + + tree payload = Backend::constructor_expression (TREE_TYPE (payload_root), + adt->is_enum (), arguments, + union_disriminator, + struct_expr.get_locus ()); + + std::vector ctor_arguments = {qualifier, payload}; translated - = Backend::constructor_expression (compiled_adt_type, adt->is_enum (), - ctor_arguments, union_disriminator, + = Backend::constructor_expression (compiled_adt_type, 0, ctor_arguments, -1, struct_expr.get_locus ()); } @@ -1247,26 +1255,34 @@ CompileExpr::visit (HIR::CallExpr &expr) arguments.push_back (rvalue); } - // the constructor depends on whether this is actually an enum or not if - // its an enum we need to setup the discriminator - std::vector ctor_arguments; - if (adt->is_enum ()) + if (!adt->is_enum ()) { - HIR::Expr &discrim_expr = variant->get_discriminant (); - tree discrim_expr_node = CompileExpr::Compile (discrim_expr, ctx); - tree folded_discrim_expr = fold_expr (discrim_expr_node); - tree qualifier = folded_discrim_expr; - - ctor_arguments.push_back (qualifier); + translated + = Backend::constructor_expression (compiled_adt_type, + adt->is_enum (), arguments, + union_disriminator, + expr.get_locus ()); + return; } - for (auto &arg : arguments) - ctor_arguments.push_back (arg); - translated - = Backend::constructor_expression (compiled_adt_type, adt->is_enum (), - ctor_arguments, union_disriminator, + HIR::Expr &discrim_expr = variant->get_discriminant (); + tree discrim_expr_node = CompileExpr::Compile (discrim_expr, ctx); + tree folded_discrim_expr = fold_expr (discrim_expr_node); + tree qualifier = folded_discrim_expr; + + tree enum_root_files = TYPE_FIELDS (compiled_adt_type); + tree payload_root = DECL_CHAIN (enum_root_files); + + tree payload + = Backend::constructor_expression (TREE_TYPE (payload_root), true, + {arguments}, union_disriminator, expr.get_locus ()); + std::vector ctor_arguments = {qualifier, payload}; + translated = Backend::constructor_expression (compiled_adt_type, false, + ctor_arguments, -1, + expr.get_locus ()); + return; } diff --git a/gcc/rust/backend/rust-compile-pattern.cc b/gcc/rust/backend/rust-compile-pattern.cc index 466afd6799a4..726ade61639a 100644 --- a/gcc/rust/backend/rust-compile-pattern.cc +++ b/gcc/rust/backend/rust-compile-pattern.cc @@ -21,6 +21,7 @@ #include "rust-compile-resolve-path.h" #include "rust-constexpr.h" #include "rust-compile-type.h" +#include "print-tree.h" namespace Rust { namespace Compile { @@ -57,11 +58,8 @@ CompilePatternCheckExpr::visit (HIR::PathInExpression &pattern) rust_assert (ok); // find discriminant field of scrutinee - tree scrutinee_record_expr - = Backend::struct_field_expression (match_scrutinee_expr, 0, - pattern.get_locus ()); tree scrutinee_expr_qualifier_expr - = Backend::struct_field_expression (scrutinee_record_expr, 0, + = Backend::struct_field_expression (match_scrutinee_expr, 0, pattern.get_locus ()); // must be enum @@ -227,11 +225,8 @@ CompilePatternCheckExpr::visit (HIR::StructPattern &pattern) tree discrim_expr_node = CompileExpr::Compile (discrim_expr, ctx); // find discriminant field of scrutinee - tree scrutinee_record_expr - = Backend::struct_field_expression (match_scrutinee_expr, variant_index, - pattern.get_path ().get_locus ()); tree scrutinee_expr_qualifier_expr - = Backend::struct_field_expression (scrutinee_record_expr, 0, + = Backend::struct_field_expression (match_scrutinee_expr, 0, pattern.get_path ().get_locus ()); check_expr @@ -240,7 +235,7 @@ CompilePatternCheckExpr::visit (HIR::StructPattern &pattern) discrim_expr_node, pattern.get_path ().get_locus ()); - match_scrutinee_expr = scrutinee_record_expr; + match_scrutinee_expr = scrutinee_expr_qualifier_expr; } else { @@ -295,8 +290,6 @@ CompilePatternCheckExpr::visit (HIR::StructPattern &pattern) void CompilePatternCheckExpr::visit (HIR::TupleStructPattern &pattern) { - size_t tuple_field_index; - // lookup the type TyTy::BaseType *lookup = nullptr; bool ok = ctx->get_tyctx ()->lookup_type ( @@ -307,6 +300,7 @@ CompilePatternCheckExpr::visit (HIR::TupleStructPattern &pattern) rust_assert (lookup->get_kind () == TyTy::TypeKind::ADT); TyTy::ADTType *adt = static_cast (lookup); + int variant_index = 0; rust_assert (adt->number_of_variants () > 0); TyTy::VariantDef *variant = nullptr; if (adt->is_enum ()) @@ -317,7 +311,6 @@ CompilePatternCheckExpr::visit (HIR::TupleStructPattern &pattern) pattern.get_path ().get_mappings ().get_hirid (), &variant_id); rust_assert (ok); - int variant_index = 0; ok = adt->lookup_variant_by_id (variant_id, &variant, &variant_index); rust_assert (ok); @@ -326,11 +319,8 @@ CompilePatternCheckExpr::visit (HIR::TupleStructPattern &pattern) tree discrim_expr_node = CompileExpr::Compile (discrim_expr, ctx); // find discriminant field of scrutinee - tree scrutinee_record_expr - = Backend::struct_field_expression (match_scrutinee_expr, variant_index, - pattern.get_path ().get_locus ()); tree scrutinee_expr_qualifier_expr - = Backend::struct_field_expression (scrutinee_record_expr, 0, + = Backend::struct_field_expression (match_scrutinee_expr, 0, pattern.get_path ().get_locus ()); check_expr @@ -338,17 +328,11 @@ CompilePatternCheckExpr::visit (HIR::TupleStructPattern &pattern) scrutinee_expr_qualifier_expr, discrim_expr_node, pattern.get_path ().get_locus ()); - - match_scrutinee_expr = scrutinee_record_expr; - // we are offsetting by + 1 here since the first field in the record - // is always the discriminator - tuple_field_index = 1; } else { variant = adt->get_variants ().at (0); check_expr = boolean_true_node; - tuple_field_index = 0; } HIR::TupleStructItems &items = pattern.get_items (); @@ -367,10 +351,20 @@ CompilePatternCheckExpr::visit (HIR::TupleStructPattern &pattern) rust_assert (items_no_range.get_patterns ().size () == variant->num_fields ()); + size_t tuple_field_index = 0; for (auto &pattern : items_no_range.get_patterns ()) { + // find payload union field of scrutinee + tree payload_ref + = Backend::struct_field_expression (match_scrutinee_expr, 1, + pattern->get_locus ()); + + tree variant_ref + = Backend::struct_field_expression (payload_ref, variant_index, + pattern->get_locus ()); + tree field_expr - = Backend::struct_field_expression (match_scrutinee_expr, + = Backend::struct_field_expression (variant_ref, tuple_field_index++, pattern->get_locus ()); @@ -470,13 +464,15 @@ CompilePatternBindings::visit (HIR::TupleStructPattern &pattern) if (adt->is_enum ()) { - // we are offsetting by + 1 here since the first field in the record - // is always the discriminator - size_t tuple_field_index = 1; + size_t tuple_field_index = 0; for (auto &pattern : items_no_range.get_patterns ()) { + tree payload_accessor_union + = Backend::struct_field_expression (match_scrutinee_expr, 1, + pattern->get_locus ()); + tree variant_accessor - = Backend::struct_field_expression (match_scrutinee_expr, + = Backend::struct_field_expression (payload_accessor_union, variant_index, pattern->get_locus ()); @@ -569,16 +565,18 @@ CompilePatternBindings::visit (HIR::StructPattern &pattern) tree binding = error_mark_node; if (adt->is_enum ()) { + tree payload_accessor_union + = Backend::struct_field_expression (match_scrutinee_expr, 1, + ident.get_locus ()); + tree variant_accessor - = Backend::struct_field_expression (match_scrutinee_expr, + = Backend::struct_field_expression (payload_accessor_union, variant_index, ident.get_locus ()); - // we are offsetting by + 1 here since the first field in the - // record is always the discriminator - binding = Backend::struct_field_expression (variant_accessor, - offs + 1, - ident.get_locus ()); + binding + = Backend::struct_field_expression (variant_accessor, offs, + ident.get_locus ()); } else { diff --git a/gcc/rust/backend/rust-compile-resolve-path.cc b/gcc/rust/backend/rust-compile-resolve-path.cc index 5f6ba7cce433..049b0d86b523 100644 --- a/gcc/rust/backend/rust-compile-resolve-path.cc +++ b/gcc/rust/backend/rust-compile-resolve-path.cc @@ -86,8 +86,9 @@ ResolvePathRef::attempt_constructor_expression_lookup ( tree folded_discrim_expr = fold_expr (discrim_expr_node); tree qualifier = folded_discrim_expr; - return Backend::constructor_expression (compiled_adt_type, true, {qualifier}, - union_disriminator, expr_locus); + // false for is enum but this is an enum but we have a new layout + return Backend::constructor_expression (compiled_adt_type, false, {qualifier}, + -1, expr_locus); } tree diff --git a/gcc/rust/backend/rust-compile-type.cc b/gcc/rust/backend/rust-compile-type.cc index 6c7bae67a1ce..34d9de477858 100644 --- a/gcc/rust/backend/rust-compile-type.cc +++ b/gcc/rust/backend/rust-compile-type.cc @@ -298,21 +298,39 @@ TyTyResolveCompile::visit (const TyTy::ADTType &type) // Ada, qual_union_types might still work for this but I am not 100% sure. // I ran into some issues lets reuse our normal union and ask Ada people // about it. + // + // I think the above is actually wrong and it should actually be this + // + // struct { + // int RUST$ENUM$DISR; // take into account the repr for this TODO + // union { + // // Variant A + // struct { + // // No additional fields + // } A; + + // // Variant B + // struct { + // // No additional fields + // } B; + + // // Variant C + // struct { + // char c; + // } C; + + // // Variant D + // struct { + // int64_t x; + // int64_t y; + // } D; + // } payload; // The union of all variant data + // }; std::vector variant_records; for (auto &variant : type.get_variants ()) { std::vector fields; - - // add in the qualifier field for the variant - tree enumeral_type - = TyTyResolveCompile::get_implicit_enumeral_node_type (); - Backend::typed_identifier f (RUST_ENUM_DISR_FIELD_NAME, enumeral_type, - ctx->get_mappings ().lookup_location ( - variant->get_id ())); - fields.push_back (std::move (f)); - - // compile the rest of the fields for (size_t i = 0; i < variant->num_fields (); i++) { const TyTy::StructFieldType *field @@ -336,9 +354,6 @@ TyTyResolveCompile::visit (const TyTy::ADTType &type) = Backend::named_type (variant->get_ident ().path.get (), variant_record, variant->get_ident ().locus); - // set the qualifier to be a builtin - DECL_ARTIFICIAL (TYPE_FIELDS (variant_record)) = 1; - // add them to the list variant_records.push_back (named_variant_record); } @@ -359,8 +374,27 @@ TyTyResolveCompile::visit (const TyTy::ADTType &type) enum_fields.push_back (std::move (f)); } + // + location_t locus = ctx->get_mappings ().lookup_location (type.get_ref ()); + // finally make the union or the enum - type_record = Backend::union_type (enum_fields, false); + tree variants_union = Backend::union_type (enum_fields, false); + layout_type (variants_union); + tree named_union_record + = Backend::named_type ("payload", variants_union, locus); + + // create the overall struct + tree enumeral_type + = TyTyResolveCompile::get_implicit_enumeral_node_type (); + Backend::typed_identifier discrim (RUST_ENUM_DISR_FIELD_NAME, + enumeral_type, locus); + Backend::typed_identifier variants_union_field ("payload", + named_union_record, + locus); + + std::vector fields + = {discrim, variants_union_field}; + type_record = Backend::struct_type (fields, false); } // Handle repr options diff --git a/gcc/rust/rust-gcc.cc b/gcc/rust/rust-gcc.cc index 3f8986e5519c..1efd872ee8f7 100644 --- a/gcc/rust/rust-gcc.cc +++ b/gcc/rust/rust-gcc.cc @@ -1352,7 +1352,7 @@ constructor_expression (tree type_tree, bool is_variant, if (!TREE_CONSTANT (elt->value)) is_constant = false; } - gcc_assert (field == NULL_TREE); + // gcc_assert (field == NULL_TREE); } }