Patchwork [gccgo] Use the same GIMPLE type for all identical Go types

login
register
mail settings
Submitter Ian Taylor
Date Nov. 10, 2010, 10:45 p.m.
Message ID <mcrd3qcvlvr.fsf@google.com>
Download mbox | patch
Permalink /patch/70707/
State New
Headers show

Comments

Ian Taylor - Nov. 10, 2010, 10:45 p.m.
The rules for type identity in Go are not the same as in GIMPLE.  In Go,
for example, two unnamed structs with the same field names and types are
identical.  In GIMPLE they are not.  This patch uses a hash table to
ensure that we produce the same GIMPLE type (the same tree pointer) for
each identical Go type, so that GIMPLE does not get confused by struct
assignments which are valid in Go but not in GIMPLE.  I previously did
that for array types, but encountered a case where it happened with a
struct type, so I moved the array code to the general Type class.
Committed to gccgo branch.

Ian

Patch

diff -r f304a0c9339a go/types.cc
--- a/go/types.cc	Wed Nov 10 14:39:09 2010 -0800
+++ b/go/types.cc	Wed Nov 10 14:41:03 2010 -0800
@@ -775,12 +775,61 @@ 
   return false;
 }
 
+// A hash table mapping unnamed types to trees.
+
+Type::Type_trees Type::type_trees;
+
 // Return a tree representing this type.
 
 tree
 Type::get_tree(Gogo* gogo)
 {
-  if (this->tree_ == NULL)
+  if (this->tree_ != NULL)
+    return this->tree_;
+
+  if (this->forward_declaration_type() != NULL
+      || this->named_type() != NULL)
+    return this->get_tree_without_hash(gogo);
+
+  // To avoid confusing GIMPLE, we need to translate all identical Go
+  // types to the same GIMPLE type.  We use a hash table to do that.
+  // There is no need to use the hash table for named types, as named
+  // types are only identical to themselves.
+
+  std::pair<Type*, tree> val(this, NULL);
+  std::pair<Type_trees::iterator, bool> ins =
+    Type::type_trees.insert(val);
+  if (!ins.second && ins.first->second != NULL_TREE)
+    {
+      this->tree_ = ins.first->second;
+      return this->tree_;
+    }
+
+  tree t = this->get_tree_without_hash(gogo);
+
+  if (ins.first->second == NULL_TREE)
+    ins.first->second = t;
+  else
+    {
+      // We have already created a tree for this type.  This can
+      // happen when an unnamed type is defined using a named type
+      // which in turns uses an identical unnamed type.  Use the tree
+      // we created earlier and ignore the one we just built.
+      t = ins.first->second;
+      this->tree_ = t;
+    }
+
+  return t;
+}
+
+// Return a tree for a type without looking in the hash table for
+// identical types.  This is used for named types, since there is no
+// point to looking in the hash table for them.
+
+tree
+Type::get_tree_without_hash(Gogo* gogo)
+{
+  if (this->tree_ == NULL_TREE)
     {
       tree t = this->do_get_tree(gogo);
 
@@ -4044,8 +4093,6 @@ 
 
 // Class Array_type.
 
-Array_type::Array_trees Array_type::array_trees;
-
 // Whether two array types are identical.
 
 bool
@@ -4320,22 +4367,6 @@ 
 {
   gcc_assert(this->length_ == NULL);
 
-  // Two different slices of the same element type are really the same
-  // type.  In order to make that valid at the tree level, we make
-  // sure to return the same struct.
-  std::pair<Type*, tree> val(this->element_type_, NULL);
-  std::pair<Array_trees::iterator, bool> ins =
-    Array_type::array_trees.insert(val);
-  if (!ins.second)
-    {
-      // We've already created a tree type for a slice with this
-      // element type.
-      gcc_assert(ins.first->second != NULL_TREE);
-      return ins.first->second;
-    }
-
-  ins.first->second = struct_type;
-
   tree element_type_tree = this->element_type_->get_tree(gogo);
   tree field = TYPE_FIELDS(struct_type);
   gcc_assert(strcmp(IDENTIFIER_POINTER(DECL_NAME(field)), "__values") == 0);
@@ -6753,7 +6784,7 @@ 
     case TYPE_MAP:
     case TYPE_CHANNEL:
       // All maps and channels have the same type in GENERIC.
-      t = this->type_->get_tree(gogo);
+      t = Type::get_named_type_tree(gogo, this->type_);
       if (t == error_mark_node)
 	return error_mark_node;
       // Build a copy to set TYPE_NAME.
@@ -6771,7 +6802,7 @@ 
 	      == this))
 	return ptr_type_node;
       this->seen_ = true;
-      t = this->type_->get_tree(gogo);
+      t = Type::get_named_type_tree(gogo, this->type_);
       this->seen_ = false;
       if (t == error_mark_node)
 	return error_mark_node;
@@ -6784,7 +6815,7 @@ 
       if (this->seen_ && this->points_to()->forwarded() == this)
 	return ptr_type_node;
       this->seen_ = true;
-      t = this->type_->get_tree(gogo);
+      t = Type::get_named_type_tree(gogo, this->type_);
       this->seen_ = false;
       if (t == error_mark_node)
 	return error_mark_node;
@@ -6801,7 +6832,7 @@ 
 
     case TYPE_ARRAY:
       if (!this->is_open_array_type())
-	t = this->type_->get_tree(gogo);
+	t = Type::get_named_type_tree(gogo, this->type_);
       else
 	{
 	  if (this->named_tree_ != NULL_TREE)
@@ -6818,7 +6849,7 @@ 
     case TYPE_INTERFACE:
       if (this->type_->interface_type()->is_empty())
 	{
-	  t = this->type_->get_tree(gogo);
+	  t = Type::get_named_type_tree(gogo, this->type_);
 	  if (t == error_mark_node)
 	    return error_mark_node;
 	  t = build_variant_type_copy(t);
@@ -6842,7 +6873,7 @@ 
 	// but it's as close as we can get with GENERIC.
 	bool was_seen = this->seen_;
 	this->seen_ = true;
-	t = this->type_->get_tree(gogo);
+	t = Type::get_named_type_tree(gogo, this->type_);
 	this->seen_ = was_seen;
 	if (was_seen)
 	  return t;
@@ -7906,7 +7937,7 @@ 
 Forward_declaration_type::do_get_tree(Gogo* gogo)
 {
   if (this->is_defined())
-    return this->real_type()->get_tree(gogo);
+    return Type::get_named_type_tree(gogo, this->real_type());
 
   if (this->warned_)
     return error_mark_node;
diff -r f304a0c9339a go/types.h
--- a/go/types.h	Wed Nov 10 14:39:09 2010 -0800
+++ b/go/types.h	Wed Nov 10 14:41:03 2010 -0800
@@ -945,6 +945,11 @@ 
   static unsigned int
   hash_string(const std::string&, unsigned int);
 
+  // Return a tree for the underlying type of a named type.
+  static tree
+  get_named_type_tree(Gogo* gogo, Type* base_type)
+  { return base_type->get_tree_without_hash(gogo); }
+
  private:
   // Convert to the desired type classification, or return NULL.  This
   // is a controlled dynamic_cast.
@@ -1057,6 +1062,18 @@ 
 		       bool* found_pointer_method,
 		       std::string* ambig1, std::string* ambig2);
 
+  // Get a tree for a type without looking in the hash table for
+  // identical types.
+  tree
+  get_tree_without_hash(Gogo*);
+
+  // A mapping from Type to tree, used to ensure that the GIMPLE
+  // representation of identical types is identical.
+  typedef std::tr1::unordered_map<const Type*, tree, Type_hash_identical,
+				  Type_identical> Type_trees;
+
+  static Type_trees type_trees;
+
   // The type classification.
   Type_classification classification_;
   // The tree representation of the type, once it has been determined.
@@ -2067,13 +2084,6 @@ 
   Expression*
   slice_type_descriptor(Gogo*, Named_type*);
 
-  // A mapping from Type to tree, used to ensure that arrays of
-  // identical types are identical.
-  typedef std::tr1::unordered_map<const Type*, tree, Type_hash_identical,
-				  Type_identical> Array_trees;
-
-  static Array_trees array_trees;
-
   // The type of elements of the array.
   Type* element_type_;
   // The number of elements.  This may be NULL.