Skip to content

Commit

Permalink
gccrs: cleanup our enum type layout to be closer to rustc
Browse files Browse the repository at this point in the history 
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 <[email protected]>
  • Loading branch information
@philberty
philberty committed Jan 10, 2025
1 parent 1eaf085 commit 806166d
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 79 deletions.
74 changes: 45 additions & 29 deletions gcc/rust/backend/rust-compile-expr.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<tree> 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<tree> 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 ());
}

Expand Down Expand Up @@ -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<tree> 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<tree> ctor_arguments = {qualifier, payload};
translated = Backend::constructor_expression (compiled_adt_type, false,
ctor_arguments, -1,
expr.get_locus ());

return;
}

Expand Down
64 changes: 31 additions & 33 deletions gcc/rust/backend/rust-compile-pattern.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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
{
Expand Down Expand Up @@ -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 (
Expand All @@ -307,6 +300,7 @@ CompilePatternCheckExpr::visit (HIR::TupleStructPattern &pattern)
rust_assert (lookup->get_kind () == TyTy::TypeKind::ADT);
TyTy::ADTType *adt = static_cast<TyTy::ADTType *> (lookup);

int variant_index = 0;
rust_assert (adt->number_of_variants () > 0);
TyTy::VariantDef *variant = nullptr;
if (adt->is_enum ())
Expand All @@ -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);

Expand All @@ -326,29 +319,20 @@ 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
= Backend::comparison_expression (ComparisonOperator::EQUAL,
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 ();
Expand All @@ -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 ());

Expand Down Expand Up @@ -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 ());

Expand Down Expand Up @@ -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
{
Expand Down
5 changes: 3 additions & 2 deletions gcc/rust/backend/rust-compile-resolve-path.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
62 changes: 48 additions & 14 deletions gcc/rust/backend/rust-compile-type.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<tree> variant_records;
for (auto &variant : type.get_variants ())
{
std::vector<Backend::typed_identifier> 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
Expand All @@ -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);
}
Expand All @@ -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<Backend::typed_identifier> fields
= {discrim, variants_union_field};
type_record = Backend::struct_type (fields, false);
}

// Handle repr options
Expand Down
2 changes: 1 addition & 1 deletion gcc/rust/rust-gcc.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

Expand Down

0 comments on commit 806166d

Please sign in to comment.