Re: Go patch committed: drop size arguments to hash/equal functions

[Ian Lance Taylor]

On Tue, Jan 10, 2017 at 11:45 AM, Rainer Orth
 wrote:
>
>> Drop the size arguments for the hash/equal functions stored in type
>> descriptors.  Types know what size they are.  To make this work,
>> generate hash/equal functions for types that can use an identity
>> comparison but are not a standard size and alignment.
>>
>> Drop the multiplications by 33 in the generated hash code and the
>> reflect package hash code.  They are not necessary since we started
>> passing a seed value around, as the seed includes the hash of the
>> earlier values.
>>
>> Copy the algorithms for standard types from the Go 1.7 runtime,
>> replacing the C functions.
>>
>> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
>> to mainline.
>
> this patch broke Solaris/SPARC bootstrap, it seems: building
> debug/dwarf.lo ICEs like this:
>
> go1: internal compiler error: in write_specific_type_functions, at 
> go/gofrontend/types.cc:1993
> 0x4bc50b Type::write_specific_type_functions(Gogo*, Named_type*, long long, 
> std::__cxx11::basic_string > const&, Function_type*, std::__cxx11::basic_string std::char_traits, std::allocator > const&, Function_type*)
> /vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/types.cc:1993

Thanks.  Looks like 32-bit SPARC requires a padding field in the
bucket struct created for some map type.  The compiler then tries to
generate type functions for the type descriptor, and fails due to a
phase ordering problem--at that point all type functions are expected
to have been created.  It worked previously because it used the
standard functions for types that can use an identity comparison, but
that is no longer the case for types with unusual sizes.  Fixed by
this patch, which marks compiler-created arrays and structs
non-comparable, meaning that no type functions are created.
Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 244291)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-d3725d876496f2cca3d6ce538e98b58c85d90bfb
+6be46149636c3533389e62c6dc76f0a7ff461080
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 244256)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -6741,8 +6741,9 @@ Bound_method_expression::create_thunk(Go
   sfl->push_back(Struct_field(Typed_identifier("val.1",
   orig_fntype->receiver()->type(),
   loc)));
-  Type* closure_type = Type::make_struct_type(sfl, loc);
-  closure_type = Type::make_pointer_type(closure_type);
+  Struct_type* st = Type::make_struct_type(sfl, loc);
+  st->set_is_struct_incomparable();
+  Type* closure_type = Type::make_pointer_type(st);
 
   Function_type* new_fntype = orig_fntype->copy_with_names();
 
@@ -6896,6 +6897,7 @@ Bound_method_expression::do_flatten(Gogo
  loc)));
   fields->push_back(Struct_field(Typed_identifier("val.1", val->type(), loc)));
   Struct_type* st = Type::make_struct_type(fields, loc);
+  st->set_is_struct_incomparable();
 
   Expression_list* vals = new Expression_list();
   vals->push_back(Expression::make_func_code_reference(thunk, loc));
@@ -9683,6 +9685,7 @@ Call_expression::do_flatten(Gogo* gogo,
 }
 
   Struct_type* st = Type::make_struct_type(sfl, loc);
+  st->set_is_struct_incomparable();
   this->call_temp_ = Statement::make_temporary(st, NULL, loc);
   inserter->insert(this->call_temp_);
 }
@@ -11565,7 +11568,8 @@ Field_reference_expression::do_lower(Gog
   Expression* length_expr = Expression::make_integer_ul(s.length(), NULL, loc);
 
   Type* byte_type = gogo->lookup_global("byte")->type_value();
-  Type* array_type = Type::make_array_type(byte_type, length_expr);
+  Array_type* array_type = Type::make_array_type(byte_type, length_expr);
+  array_type->set_is_array_incomparable();
 
   Expression_list* bytes = new Expression_list();
   for (std::string::const_iterator p = s.begin(); p != s.end(); p++)
@@ -11843,8 +11847,9 @@ Interface_field_reference_expression::cr
   Type* vt = Type::make_pointer_type(Type::make_void_type());
   sfl->push_back(Struct_field(Typed_identifier("fn.0", vt, loc)));
   sfl->push_back(Struct_field(Typed_identifier("val.1", type, loc)));
-  Type* closure_type = Type::make_struct_type(sfl, loc);
-  closure_type = Type::make_pointer_type(closure_type);
+  Struct_type* st = Type::make_struct_type(sfl, loc);
+  st->set_is_struct_incomparable();
+  Type* closure_type = Type::make_pointer_type(st);
 
   Function_type* 

Re: Go patch committed: drop size arguments to hash/equal functions

[Rainer Orth]

Hi Ian,

> Drop the size arguments for the hash/equal functions stored in type
> descriptors.  Types know what size they are.  To make this work,
> generate hash/equal functions for types that can use an identity
> comparison but are not a standard size and alignment.
>
> Drop the multiplications by 33 in the generated hash code and the
> reflect package hash code.  They are not necessary since we started
> passing a seed value around, as the seed includes the hash of the
> earlier values.
>
> Copy the algorithms for standard types from the Go 1.7 runtime,
> replacing the C functions.
>
> Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
> to mainline.

this patch broke Solaris/SPARC bootstrap, it seems: building
debug/dwarf.lo ICEs like this:

go1: internal compiler error: in write_specific_type_functions, at 
go/gofrontend/types.cc:1993
0x4bc50b Type::write_specific_type_functions(Gogo*, Named_type*, long long, 
std::__cxx11::basic_string 
const&, Function_type*, std::__cxx11::basic_string const&, Function_type*)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/types.cc:1993
0x4bc8ff Type::specific_type_functions(Gogo*, Named_type*, long long, 
Function_type*, Function_type*, Named_object**, Named_object**)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/types.cc:1971
0x4bb2a3 Type::type_functions(Gogo*, Named_type*, Function_type*, 
Function_type*, Named_object**, Named_object**)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/types.cc:1753
0x4bf06f Type::type_descriptor_constructor(Gogo*, int, Named_type*, Methods 
const*, bool)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/types.cc:2308
0x4c0893 Array_type::array_type_descriptor(Gogo*, Named_type*)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/types.cc:6932
0x4b43c7 Type::make_type_descriptor_var(Gogo*)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/types.cc:1237
0x4b47c3 Type::type_descriptor_pointer(Gogo*, Location)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/types.cc:1172
0x1232d63 Type_descriptor_expression::do_get_backend(Translate_context*)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/expressions.cc:14344
0x4251df Expression::get_backend(Translate_context*)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/expressions.cc:402
0x43faaf Struct_construction_expression::do_get_backend(Translate_context*)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/expressions.cc:12572
0x4251df Expression::get_backend(Translate_context*)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/expressions.cc:402
0x4408d3 Array_construction_expression::get_constructor(Translate_context*, 
Btype*)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/expressions.cc:12795
0x4251df Expression::get_backend(Translate_context*)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/expressions.cc:402
0x43cb93 Unary_expression::do_get_backend(Translate_context*)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/expressions.cc:4249
0x4251df Expression::get_backend(Translate_context*)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/expressions.cc:402
0x4260ef Slice_value_expression::do_get_backend(Translate_context*)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/expressions.cc:14729
0x4251df Expression::get_backend(Translate_context*)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/expressions.cc:402
0x4251df Expression::get_backend(Translate_context*)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/expressions.cc:402
0x43faaf Struct_construction_expression::do_get_backend(Translate_context*)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/expressions.cc:12572
0x4251df Expression::get_backend(Translate_context*)
/vol/gcc/src/hg/trunk/local/gcc/go/gofrontend/expressions.cc:402

Solaris/x86 is fine.

Rainer

-- 
-
Rainer Orth, Center for Biotechnology, Bielefeld University

Go patch committed: drop size arguments to hash/equal functions

[Ian Lance Taylor]

Drop the size arguments for the hash/equal functions stored in type
descriptors.  Types know what size they are.  To make this work,
generate hash/equal functions for types that can use an identity
comparison but are not a standard size and alignment.

Drop the multiplications by 33 in the generated hash code and the
reflect package hash code.  They are not necessary since we started
passing a seed value around, as the seed includes the hash of the
earlier values.

Copy the algorithms for standard types from the Go 1.7 runtime,
replacing the C functions.

Bootstrapped and ran Go testsuite on x86_64-pc-linux-gnu.  Committed
to mainline.

Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 244236)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-189ea81cc758e000325fd6cca7882c252d33f8f0
+f439989e483b7c2eada6ddcf6e730a791cce603f
 
 The first line of this file holds the git revision number of the last
 merge done from the gofrontend repository.
Index: gcc/go/gofrontend/expressions.cc
===
--- gcc/go/gofrontend/expressions.cc(revision 244166)
+++ gcc/go/gofrontend/expressions.cc(working copy)
@@ -5335,7 +5335,6 @@ Binary_expression::lower_array_compariso
   Expression_list* args = new Expression_list();
   args->push_back(this->operand_address(inserter, this->left_));
   args->push_back(this->operand_address(inserter, this->right_));
-  args->push_back(Expression::make_type_info(at, TYPE_INFO_SIZE));
 
   Expression* ret = Expression::make_call(func, args, false, loc);
 
Index: gcc/go/gofrontend/gogo.cc
===
--- gcc/go/gofrontend/gogo.cc   (revision 244166)
+++ gcc/go/gofrontend/gogo.cc   (working copy)
@@ -2343,7 +2343,7 @@ Gogo::clear_file_scope()
 // parse tree is lowered.
 
 void
-Gogo::queue_specific_type_function(Type* type, Named_type* name,
+Gogo::queue_specific_type_function(Type* type, Named_type* name, int64_t size,
   const std::string& hash_name,
   Function_type* hash_fntype,
   const std::string& equal_name,
@@ -2351,7 +2351,7 @@ Gogo::queue_specific_type_function(Type*
 {
   go_assert(!this->specific_type_functions_are_written_);
   go_assert(!this->in_global_scope());
-  Specific_type_function* tsf = new Specific_type_function(type, name,
+  Specific_type_function* tsf = new Specific_type_function(type, name, size,
   hash_name,
   hash_fntype,
   equal_name,
@@ -2386,7 +2386,7 @@ Specific_type_functions::type(Type* t)
 case Type::TYPE_NAMED:
   {
Named_type* nt = t->named_type();
-   if (!t->compare_is_identity(this->gogo_) && t->is_comparable())
+   if (t->needs_specific_type_functions(this->gogo_))
  t->type_functions(this->gogo_, nt, NULL, NULL, _fn, _fn);
 
// If this is a struct type, we don't want to make functions
@@ -2420,7 +2420,7 @@ Specific_type_functions::type(Type* t)
 
 case Type::TYPE_STRUCT:
 case Type::TYPE_ARRAY:
-  if (!t->compare_is_identity(this->gogo_) && t->is_comparable())
+  if (t->needs_specific_type_functions(this->gogo_))
t->type_functions(this->gogo_, NULL, NULL, NULL, _fn, _fn);
   break;
 
@@ -2443,7 +2443,7 @@ Gogo::write_specific_type_functions()
 {
   Specific_type_function* tsf = this->specific_type_functions_.back();
   this->specific_type_functions_.pop_back();
-  tsf->type->write_specific_type_functions(this, tsf->name,
+  tsf->type->write_specific_type_functions(this, tsf->name, tsf->size,
   tsf->hash_name,
   tsf->hash_fntype,
   tsf->equal_name,
Index: gcc/go/gofrontend/gogo.h
===
--- gcc/go/gofrontend/gogo.h(revision 244166)
+++ gcc/go/gofrontend/gogo.h(working copy)
@@ -563,7 +563,7 @@ class Gogo
   // used when a type-specific function is needed when not at the top
   // level.
   void
-  queue_specific_type_function(Type* type, Named_type* name,
+  queue_specific_type_function(Type* type, Named_type* name, int64_t size,
   const std::string& hash_name,
   Function_type* hash_fntype,
   const std::string& equal_name,
@@ -824,17 +824,18 @@ class Gogo
   {
 Type* type;
 Named_type* name;
+int64_t size;
 std::string hash_name;
 Function_type* hash_fntype;
 std::string equal_name;
 Function_type* equal_fntype;
 
-Specific_type_function(Type* atype,