Go patch committed: Avoid copy for string([]byte) in string comparison

Ian Lance Taylor iant@golang.org
Wed May 8 23:08:00 GMT 2019


This Go frontend patch by Cherry Zhang avoids a copy for a
string([]byte) conversion used in a string comparison.  If a
string([]byte) conversion is used immediately in a string comparison,
we don't need to copy the backing store of the byte slice, as the
string comparison doesn't hold any reference to it.  Instead, just
create a string header from the byte slice and pass it for comparison.
A new type of expression, String_value_expression, is introduced, for
constructing string headers.  Bootstrapped and ran Go testsuite on
x86_64-pc-linux-gnu.  Committed to mainline.

Ian

2019-05-08  Cherry Zhang  <cherryyz@google.com>

* go.dg/cmpstring.go: New test.
-------------- next part --------------
Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE	(revision 271019)
+++ gcc/go/gofrontend/MERGE	(working copy)
@@ -1,4 +1,4 @@
-f813c670deb8e0454c3f64de74bedb5dcedd10b4
+9c8581187b1c1a30036263728370f31cb846a274
 
 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 271017)
+++ gcc/go/gofrontend/expressions.cc	(working copy)
@@ -2031,6 +2031,90 @@ Expression::make_string_info(Expression*
   return new String_info_expression(string, string_info, location);
 }
 
+// An expression that represents an string value: a struct with value pointer
+// and length fields.
+
+class String_value_expression : public Expression
+{
+ public:
+  String_value_expression(Expression* valptr, Expression* len, Location location)
+      : Expression(EXPRESSION_STRING_VALUE, location),
+        valptr_(valptr), len_(len)
+  { }
+
+ protected:
+  int
+  do_traverse(Traverse*);
+
+  Type*
+  do_type()
+  { return Type::make_string_type(); }
+
+  void
+  do_determine_type(const Type_context*)
+  { go_unreachable(); }
+
+  Expression*
+  do_copy()
+  {
+    return new String_value_expression(this->valptr_->copy(),
+                                       this->len_->copy(),
+                                       this->location());
+  }
+
+  Bexpression*
+  do_get_backend(Translate_context* context);
+
+  void
+  do_dump_expression(Ast_dump_context*) const;
+
+ private:
+  // The value pointer.
+  Expression* valptr_;
+  // The length.
+  Expression* len_;
+};
+
+int
+String_value_expression::do_traverse(Traverse* traverse)
+{
+  if (Expression::traverse(&this->valptr_, traverse) == TRAVERSE_EXIT
+      || Expression::traverse(&this->len_, traverse) == TRAVERSE_EXIT)
+    return TRAVERSE_EXIT;
+  return TRAVERSE_CONTINUE;
+}
+
+Bexpression*
+String_value_expression::do_get_backend(Translate_context* context)
+{
+  std::vector<Bexpression*> vals(2);
+  vals[0] = this->valptr_->get_backend(context);
+  vals[1] = this->len_->get_backend(context);
+
+  Gogo* gogo = context->gogo();
+  Btype* btype = Type::make_string_type()->get_backend(gogo);
+  return gogo->backend()->constructor_expression(btype, vals, this->location());
+}
+
+void
+String_value_expression::do_dump_expression(
+    Ast_dump_context* ast_dump_context) const
+{
+  ast_dump_context->ostream() << "stringvalue(";
+  ast_dump_context->ostream() << "value: ";
+  this->valptr_->dump_expression(ast_dump_context);
+  ast_dump_context->ostream() << ", length: ";
+  this->len_->dump_expression(ast_dump_context);
+  ast_dump_context->ostream() << ")";
+}
+
+Expression*
+Expression::make_string_value(Expression* valptr, Expression* len,
+                              Location location)
+{
+  return new String_value_expression(valptr, len, location);
+}
+
 // Make an integer expression.
 
 class Integer_expression : public Expression
@@ -3702,9 +3786,11 @@ Type_conversion_expression::do_check_typ
 Expression*
 Type_conversion_expression::do_copy()
 {
-  return new Type_conversion_expression(this->type_->copy_expressions(),
-					this->expr_->copy(),
-					this->location());
+  Expression* ret = new Type_conversion_expression(this->type_->copy_expressions(),
+                                                   this->expr_->copy(),
+                                                   this->location());
+  ret->conversion_expression()->set_no_copy(this->no_copy_);
+  return ret;
 }
 
 // Get the backend representation for a type conversion.
@@ -3764,7 +3850,22 @@ Type_conversion_expression::do_get_backe
 
       Runtime::Function code;
       if (e->integer_type()->is_byte())
-        code = Runtime::SLICEBYTETOSTRING;
+        {
+          if (this->no_copy_)
+            {
+              if (gogo->debug_optimization())
+                go_inform(loc, "no copy string([]byte)");
+              Expression* ptr = Expression::make_slice_info(this->expr_,
+                                                            SLICE_INFO_VALUE_POINTER,
+                                                            loc);
+              Expression* len = Expression::make_slice_info(this->expr_,
+                                                            SLICE_INFO_LENGTH,
+                                                            loc);
+              Expression* str = Expression::make_string_value(ptr, len, loc);
+              return str->get_backend(context);
+            }
+          code = Runtime::SLICEBYTETOSTRING;
+        }
       else
         {
           go_assert(e->integer_type()->is_rune());
@@ -6805,6 +6906,15 @@ Expression::comparison(Translate_context
 
   if (left_type->is_string_type() && right_type->is_string_type())
     {
+      // Mark string([]byte) operands to reuse the backing store.
+      // String comparison does not keep the reference, so it is safe.
+      Type_conversion_expression* lce = left->conversion_expression();
+      if (lce != NULL && lce->expr()->type()->is_slice_type())
+        lce->set_no_copy(true);
+      Type_conversion_expression* rce = right->conversion_expression();
+      if (rce != NULL && rce->expr()->type()->is_slice_type())
+        rce->set_no_copy(true);
+
       if (op == OPERATOR_EQEQ || op == OPERATOR_NOTEQ)
 	{
 	  left = Runtime::make_call(Runtime::EQSTRING, location, 2,
Index: gcc/go/gofrontend/expressions.h
===================================================================
--- gcc/go/gofrontend/expressions.h	(revision 271014)
+++ gcc/go/gofrontend/expressions.h	(working copy)
@@ -102,6 +102,7 @@ class Expression
     EXPRESSION_BOOLEAN,
     EXPRESSION_STRING,
     EXPRESSION_STRING_INFO,
+    EXPRESSION_STRING_VALUE,
     EXPRESSION_INTEGER,
     EXPRESSION_FLOAT,
     EXPRESSION_COMPLEX,
@@ -248,6 +249,10 @@ class Expression
   static Expression*
   make_string_info(Expression* string, String_info, Location);
 
+  // Make an expression for a string value.
+  static Expression*
+  make_string_value(Expression* valptr, Expression* len, Location);
+
   // Make a character constant expression.  TYPE should be NULL for an
   // abstract type.
   static Expression*
@@ -1668,7 +1673,8 @@ class Type_conversion_expression : publi
   Type_conversion_expression(Type* type, Expression* expr,
 			     Location location)
     : Expression(EXPRESSION_CONVERSION, location),
-      type_(type), expr_(expr), may_convert_function_types_(false)
+      type_(type), expr_(expr), may_convert_function_types_(false),
+      no_copy_(false)
   { }
 
   // Return the type to which we are converting.
@@ -1689,6 +1695,12 @@ class Type_conversion_expression : publi
     this->may_convert_function_types_ = true;
   }
 
+  // Mark string([]byte) conversion to reuse the backing store
+  // without copying.
+  void
+  set_no_copy(bool b)
+  { this->no_copy_ = b; };
+
   // Import a type conversion expression.
   static Expression*
   do_import(Import_expression*, Location);
@@ -1751,6 +1763,9 @@ class Type_conversion_expression : publi
   // True if this is permitted to convert function types.  This is
   // used internally for method expressions.
   bool may_convert_function_types_;
+  // True if a string([]byte) conversion can reuse the backing store
+  // without copying.  Only used in string([]byte) conversion.
+  bool no_copy_;
 };
 
 // An unsafe type conversion, used to pass values to builtin functions.
Index: gcc/testsuite/go.dg/cmpstring.go
===================================================================
--- gcc/testsuite/go.dg/cmpstring.go	(nonexistent)
+++ gcc/testsuite/go.dg/cmpstring.go	(working copy)
@@ -0,0 +1,13 @@
+// { dg-do compile }
+// { dg-options "-fgo-debug-optimization" }
+
+package p
+
+func F(x []byte, y string) bool {
+	return string(x) == y // { dg-error "no copy string\\(\\\[\\\]byte\\)" }
+}
+
+func BytesEqual(x, y []byte) bool {
+	return string(x) == // { dg-error "no copy string\\(\\\[\\\]byte\\)" }
+		string(y)   // { dg-error "no copy string\\(\\\[\\\]byte\\)" }
+}


More information about the Gcc-patches mailing list