Ulrich Weigand pointed out that the Go frontend is generating invalid
GIMPLE for a call to an interface method: the function type does not
match the number of arguments passed. I'm surprised this works at all,
but in any case this patch fixes it.
Fixing this problem showed that the frontend was not correctly handling
function pointers. I changed the function pointer representation a few
months back so that a Go function is represented as a pointer to a
struct. This updates the calls to the backend interface so that
placeholder pointers for Go functions are not marked as C functions.
Also in order to avoid introducing unnecessary conversions between
identical struct types, this patch unifies the return type of all
functions with the same result parameters. Using a single backend
representation for them lets the compiler not worry about converting
that result struct to an identical one.
Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu.
Committed to mainline.
Ian
2013-11-23 Ian Lance Taylor <i...@google.com>
* go-gcc.cc (Gcc_backend::function_type): Add result_struct
parameter.
Index: gcc/go/gofrontend/expressions.cc
===================================================================
--- gcc/go/gofrontend/expressions.cc (revision 205274)
+++ gcc/go/gofrontend/expressions.cc (working copy)
@@ -9863,8 +9863,11 @@ Call_expression::do_get_tree(Translate_c
fndecl = TREE_OPERAND(fndecl, 0);
// Add a type cast in case the type of the function is a recursive
- // type which refers to itself.
- if (!DECL_P(fndecl) || !DECL_IS_BUILTIN(fndecl))
+ // type which refers to itself. We don't do this for an interface
+ // method because 1) an interface method never refers to itself, so
+ // we always have a function type here; 2) we pass an extra first
+ // argument to an interface method, so fnfield_type is not correct.
+ if ((!DECL_P(fndecl) || !DECL_IS_BUILTIN(fndecl)) && !is_interface_method)
fn = fold_convert_loc(location.gcc_location(), fnfield_type, fn);
// This is to support builtin math functions when using 80387 math.
Index: gcc/go/gofrontend/backend.h
===================================================================
--- gcc/go/gofrontend/backend.h (revision 205274)
+++ gcc/go/gofrontend/backend.h (working copy)
@@ -101,11 +101,15 @@ class Backend
// is provided so that the names are available. This should return
// not the type of a Go function (which is a pointer to a struct)
// but the type of a C function pointer (which will be used as the
- // type of the first field of the struct).
+ // type of the first field of the struct). If there is more than
+ // one result, RESULT_STRUCT is a struct type to hold the results,
+ // and RESULTS may be ignored; if there are zero or one results,
+ // RESULT_STRUCT is NULL.
virtual Btype*
function_type(const Btyped_identifier& receiver,
const std::vector<Btyped_identifier>& parameters,
const std::vector<Btyped_identifier>& results,
+ Btype* result_struct,
Location location) = 0;
// Get a struct type.
@@ -121,10 +125,11 @@ class Backend
// NAME is the name of the type, and the location is where the named
// type is defined. This function is also used for unnamed function
// types with multiple results, in which case the type has no name
- // and NAME will be empty. FOR_FUNCTION is true if this is for a Go
- // function type, which corresponds to a C/C++ pointer to function
- // type. The return value will later be passed as the first
- // parameter to set_placeholder_pointer_type or
+ // and NAME will be empty. FOR_FUNCTION is true if this is for a C
+ // pointer to function type. A Go func type is represented as a
+ // pointer to a struct, and the first field of the struct is a C
+ // pointer to function. The return value will later be passed as
+ // the first parameter to set_placeholder_pointer_type or
// set_placeholder_function_type.
virtual Btype*
placeholder_pointer_type(const std::string& name, Location,
Index: gcc/go/gofrontend/types.cc
===================================================================
--- gcc/go/gofrontend/types.cc (revision 205274)
+++ gcc/go/gofrontend/types.cc (working copy)
@@ -1059,8 +1059,9 @@ Type::get_backend_placeholder(Gogo* gogo
{
case TYPE_FUNCTION:
{
+ // A Go function type is a pointer to a struct type.
Location loc = this->function_type()->location();
- bt = gogo->backend()->placeholder_pointer_type("", loc, true);
+ bt = gogo->backend()->placeholder_pointer_type("", loc, false);
}
break;
@@ -1153,7 +1154,7 @@ Type::finish_backend(Gogo* gogo, Btype *
case TYPE_FUNCTION:
{
Btype* bt = this->do_get_backend(gogo);
- if (!gogo->backend()->set_placeholder_function_type(placeholder, bt))
+ if (!gogo->backend()->set_placeholder_pointer_type(placeholder, bt))
go_assert(saw_errors());
}
break;
@@ -3378,6 +3379,48 @@ Function_type::do_hash_for_method(Gogo*
return ret;
}
+// Hash result parameters.
+
+unsigned int
+Function_type::Results_hash::operator()(const Typed_identifier_list* t) const
+{
+ unsigned int hash = 0;
+ for (Typed_identifier_list::const_iterator p = t->begin();
+ p != t->end();
+ ++p)
+ {
+ hash <<= 2;
+ hash = Type::hash_string(p->name(), hash);
+ hash += p->type()->hash_for_method(NULL);
+ }
+ return hash;
+}
+
+// Compare result parameters so that can map identical result
+// parameters to a single struct type.
+
+bool
+Function_type::Results_equal::operator()(const Typed_identifier_list* a,
+ const Typed_identifier_list* b) const
+{
+ if (a->size() != b->size())
+ return false;
+ Typed_identifier_list::const_iterator pa = a->begin();
+ for (Typed_identifier_list::const_iterator pb = b->begin();
+ pb != b->end();
+ ++pa, ++pb)
+ {
+ if (pa->name() != pb->name()
+ || !Type::are_identical(pa->type(), pb->type(), true, NULL))
+ return false;
+ }
+ return true;
+}
+
+// Hash from results to a backend struct type.
+
+Function_type::Results_structs Function_type::results_structs;
+
// Get the backend representation for a function type.
Btype*
@@ -3416,12 +3459,14 @@ Function_type::get_backend_fntype(Gogo*
}
std::vector<Backend::Btyped_identifier> bresults;
+ Btype* bresult_struct = NULL;
if (this->results_ != NULL)
{
bresults.resize(this->results_->size());
size_t i = 0;
for (Typed_identifier_list::const_iterator p =
- this->results_->begin(); p != this->results_->end();
+ this->results_->begin();
+ p != this->results_->end();
++p, ++i)
{
bresults[i].name = Gogo::unpack_hidden_name(p->name());
@@ -3429,10 +3474,42 @@ Function_type::get_backend_fntype(Gogo*
bresults[i].location = p->location();
}
go_assert(i == bresults.size());
+
+ if (this->results_->size() > 1)
+ {
+ // Use the same results struct for all functions that
+ // return the same set of results. This is useful to
+ // unify calls to interface methods with other calls.
+ std::pair<Typed_identifier_list*, Btype*> val;
+ val.first = this->results_;
+ val.second = NULL;
+ std::pair<Results_structs::iterator, bool> ins =
+ Function_type::results_structs.insert(val);
+ if (ins.second)
+ {
+ // Build a new struct type.
+ Struct_field_list* sfl = new Struct_field_list;
+ for (Typed_identifier_list::const_iterator p =
+ this->results_->begin();
+ p != this->results_->end();
+ ++p)
+ {
+ Typed_identifier tid = *p;
+ if (tid.name().empty())
+ tid = Typed_identifier("UNNAMED", tid.type(),
+ tid.location());
+ sfl->push_back(Struct_field(tid));
+ }
+ Struct_type* st = Type::make_struct_type(sfl,
+ this->location());
+ ins.first->second = st->get_backend(gogo);
+ }
+ bresult_struct = ins.first->second;
+ }
}
this->fnbtype_ = gogo->backend()->function_type(breceiver, bparameters,
- bresults,
+ bresults, bresult_struct,
this->location());
}
@@ -7134,18 +7211,18 @@ Interface_type::get_backend_empty_interf
return empty_interface_type;
}
-// Return the fields of a non-empty interface type. This is not
-// declared in types.h so that types.h doesn't have to #include
-// backend.h.
+// Return a pointer to the backend representation of the method table.
-static void
-get_backend_interface_fields(Gogo* gogo, Interface_type* type,
- bool use_placeholder,
- std::vector<Backend::Btyped_identifier>* bfields)
+Btype*
+Interface_type::get_backend_methods(Gogo* gogo)
{
- Location loc = type->location();
+ if (this->bmethods_ != NULL && !this->bmethods_is_placeholder_)
+ return this->bmethods_;
+
+ Location loc = this->location();
- std::vector<Backend::Btyped_identifier> mfields(type->methods()->size() + 1);
+ std::vector<Backend::Btyped_identifier>
+ mfields(this->all_methods_->size() + 1);
Type* pdt = Type::make_type_descriptor_ptr_type();
mfields[0].name = "__type_descriptor";
@@ -7154,8 +7231,8 @@ get_backend_interface_fields(Gogo* gogo,
std::string last_name = "";
size_t i = 1;
- for (Typed_identifier_list::const_iterator p = type->methods()->begin();
- p != type->methods()->end();
+ for (Typed_identifier_list::const_iterator p = this->all_methods_->begin();
+ p != this->all_methods_->end();
++p, ++i)
{
// The type of the method in Go only includes the parameters.
@@ -7186,21 +7263,56 @@ get_backend_interface_fields(Gogo* gogo,
ft->location());
mfields[i].name = Gogo::unpack_hidden_name(p->name());
- mfields[i].btype = (use_placeholder
- ? mft->get_backend_placeholder(gogo)
- : mft->get_backend(gogo));
+ mfields[i].btype = mft->get_backend_fntype(gogo);
mfields[i].location = loc;
+
// Sanity check: the names should be sorted.
go_assert(p->name() > last_name);
last_name = p->name();
}
- Btype* methods = gogo->backend()->struct_type(mfields);
+ Btype* st = gogo->backend()->struct_type(mfields);
+ Btype* ret = gogo->backend()->pointer_type(st);
+
+ if (this->bmethods_ != NULL && this->bmethods_is_placeholder_)
+ gogo->backend()->set_placeholder_pointer_type(this->bmethods_, ret);
+ this->bmethods_ = ret;
+ this->bmethods_is_placeholder_ = false;
+ return ret;
+}
+
+// Return a placeholder for the pointer to the backend methods table.
+
+Btype*
+Interface_type::get_backend_methods_placeholder(Gogo* gogo)
+{
+ if (this->bmethods_ == NULL)
+ {
+ Location loc = this->location();
+ this->bmethods_ = gogo->backend()->placeholder_pointer_type("", loc,
+ false);
+ this->bmethods_is_placeholder_ = true;
+ }
+ return this->bmethods_;
+}
+
+// Return the fields of a non-empty interface type. This is not
+// declared in types.h so that types.h doesn't have to #include
+// backend.h.
+
+static void
+get_backend_interface_fields(Gogo* gogo, Interface_type* type,
+ bool use_placeholder,
+ std::vector<Backend::Btyped_identifier>* bfields)
+{
+ Location loc = type->location();
bfields->resize(2);
(*bfields)[0].name = "__methods";
- (*bfields)[0].btype = gogo->backend()->pointer_type(methods);
+ (*bfields)[0].btype = (use_placeholder
+ ? type->get_backend_methods_placeholder(gogo)
+ : type->get_backend_methods(gogo));
(*bfields)[0].location = loc;
Type* vt = Type::make_pointer_type(Type::make_void_type());
@@ -7241,7 +7353,7 @@ Interface_type::do_get_backend(Gogo* gog
void
Interface_type::finish_backend_methods(Gogo* gogo)
{
- if (!this->interface_type()->is_empty())
+ if (!this->is_empty())
{
const Typed_identifier_list* methods = this->methods();
if (methods != NULL)
@@ -7251,6 +7363,10 @@ Interface_type::finish_backend_methods(G
++p)
p->type()->get_backend(gogo);
}
+
+ // Getting the backend methods now will set the placeholder
+ // pointer.
+ this->get_backend_methods(gogo);
}
}
@@ -8542,14 +8658,14 @@ Named_type::do_get_backend(Gogo* gogo)
if (this->seen_in_get_backend_)
{
this->is_circular_ = true;
- return gogo->backend()->circular_pointer_type(bt, true);
+ return gogo->backend()->circular_pointer_type(bt, false);
}
this->seen_in_get_backend_ = true;
bt1 = Type::get_named_base_btype(gogo, base);
this->seen_in_get_backend_ = false;
if (this->is_circular_)
- bt1 = gogo->backend()->circular_pointer_type(bt, true);
- if (!gogo->backend()->set_placeholder_function_type(bt, bt1))
+ bt1 = gogo->backend()->circular_pointer_type(bt, false);
+ if (!gogo->backend()->set_placeholder_pointer_type(bt, bt1))
bt = gogo->backend()->error_type();
return bt;
Index: gcc/go/gofrontend/types.h
===================================================================
--- gcc/go/gofrontend/types.h (revision 205274)
+++ gcc/go/gofrontend/types.h (working copy)
@@ -1840,6 +1840,27 @@ class Function_type : public Type
type_descriptor_params(Type*, const Typed_identifier*,
const Typed_identifier_list*);
+ // A mapping from a list of result types to a backend struct type.
+ class Results_hash
+ {
+ public:
+ unsigned int
+ operator()(const Typed_identifier_list*) const;
+ };
+
+ class Results_equal
+ {
+ public:
+ bool
+ operator()(const Typed_identifier_list*,
+ const Typed_identifier_list*) const;
+ };
+
+ typedef Unordered_map_hash(Typed_identifier_list*, Btype*,
+ Results_hash, Results_equal) Results_structs;
+
+ static Results_structs results_structs;
+
// The receiver name and type. This will be NULL for a normal
// function, non-NULL for a method.
Typed_identifier* receiver_;
@@ -2552,8 +2573,9 @@ class Interface_type : public Type
Interface_type(Typed_identifier_list* methods, Location location)
: Type(TYPE_INTERFACE),
parse_methods_(methods), all_methods_(NULL), location_(location),
- interface_btype_(NULL), assume_identical_(NULL),
- methods_are_finalized_(false), seen_(false)
+ interface_btype_(NULL), bmethods_(NULL), assume_identical_(NULL),
+ methods_are_finalized_(false), bmethods_is_placeholder_(false),
+ seen_(false)
{ go_assert(methods == NULL || !methods->empty()); }
// The location where the interface type was defined.
@@ -2620,6 +2642,15 @@ class Interface_type : public Type
static Btype*
get_backend_empty_interface_type(Gogo*);
+ // Get a pointer to the backend representation of the method table.
+ Btype*
+ get_backend_methods(Gogo*);
+
+ // Return a placeholder for the backend representation of the
+ // pointer to the method table.
+ Btype*
+ get_backend_methods_placeholder(Gogo*);
+
// Finish the backend representation of the method types.
void
finish_backend_methods(Gogo*);
@@ -2686,11 +2717,15 @@ class Interface_type : public Type
Location location_;
// The backend representation of this type during backend conversion.
Btype* interface_btype_;
+ // The backend representation of the pointer to the method table.
+ Btype* bmethods_;
// A list of interface types assumed to be identical during
// interface comparison.
mutable Assume_identical* assume_identical_;
// Whether the methods have been finalized.
bool methods_are_finalized_;
+ // Whether the bmethods_ field is a placeholder.
+ bool bmethods_is_placeholder_;
// Used to avoid endless recursion in do_mangled_name.
mutable bool seen_;
};
Index: gcc/go/go-gcc.cc
===================================================================
--- gcc/go/go-gcc.cc (revision 205274)
+++ gcc/go/go-gcc.cc (working copy)
@@ -158,6 +158,7 @@ class Gcc_backend : public Backend
function_type(const Btyped_identifier&,
const std::vector<Btyped_identifier>&,
const std::vector<Btyped_identifier>&,
+ Btype*,
const Location);
Btype*
@@ -493,7 +494,8 @@ Btype*
Gcc_backend::function_type(const Btyped_identifier& receiver,
const std::vector<Btyped_identifier>& parameters,
const std::vector<Btyped_identifier>& results,
- Location location)
+ Btype* result_struct,
+ Location)
{
tree args = NULL_TREE;
tree* pp = &args;
@@ -528,29 +530,8 @@ Gcc_backend::function_type(const Btyped_
result = results.front().btype->get_tree();
else
{
- result = make_node(RECORD_TYPE);
- tree field_trees = NULL_TREE;
- pp = &field_trees;
- for (std::vector<Btyped_identifier>::const_iterator p = results.begin();
- p != results.end();
- ++p)
- {
- const std::string name = (p->name.empty()
- ? "UNNAMED"
- : p->name);
- tree name_tree = get_identifier_from_string(name);
- tree field_type_tree = p->btype->get_tree();
- if (field_type_tree == error_mark_node)
- return this->error_type();
- gcc_assert(TYPE_SIZE(field_type_tree) != NULL_TREE);
- tree field = build_decl(location.gcc_location(), FIELD_DECL,
- name_tree, field_type_tree);
- DECL_CONTEXT(field) = result;
- *pp = field;
- pp = &DECL_CHAIN(field);
- }
- TYPE_FIELDS(result) = field_trees;
- layout_type(result);
+ gcc_assert(result_struct != NULL);
+ result = result_struct->get_tree();
}
if (result == error_mark_node)
return this->error_type();
