Go patch committed: Intrinsify runtime/internal/atomic functions

Go patch committed: Intrinsify runtime/internal/atomic functions

[Ian Lance Taylor]

[-- Attachment #1: Type: text/plain, Size: 498 bytes --]

This patch to the Go frontend by Cherry Zhang intrinsifies the
runtime/internal/atomic functions.  Currently the
runtime/internal/atomic functions are implemented in C using C
compiler intrinsics.  This patch lets the Go frontend recognize these
functions and turn them into intrinsics directly.  Bootstrapped and
ran Go testsuite on x86_64-pc-linux-gnu.  Committed to mainline.

Ian

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

* go-gcc.cc (Gcc_backend::Gcc_backend): Define atomic builtins.

[-- Attachment #2: patch.txt --]
[-- Type: text/plain, Size: 18734 bytes --]

Index: gcc/go/go-gcc.cc
===================================================================
--- gcc/go/go-gcc.cc	(revision 271182)
+++ gcc/go/go-gcc.cc	(working copy)
@@ -776,6 +776,109 @@ Gcc_backend::Gcc_backend()
   this->define_builtin(BUILT_IN_UNREACHABLE, "__builtin_unreachable", NULL,
 		       build_function_type(void_type_node, void_list_node),
 		       true, true);
+
+  // We provide some atomic functions.
+  t = build_function_type_list(uint32_type_node,
+                               ptr_type_node,
+                               integer_type_node,
+                               NULL_TREE);
+  this->define_builtin(BUILT_IN_ATOMIC_LOAD_4, "__atomic_load_4", NULL,
+                       t, false, false);
+
+  t = build_function_type_list(uint64_type_node,
+                               ptr_type_node,
+                               integer_type_node,
+                               NULL_TREE);
+  this->define_builtin(BUILT_IN_ATOMIC_LOAD_8, "__atomic_load_8", NULL,
+                       t, false, false);
+
+  t = build_function_type_list(void_type_node,
+                               ptr_type_node,
+                               uint32_type_node,
+                               integer_type_node,
+                               NULL_TREE);
+  this->define_builtin(BUILT_IN_ATOMIC_STORE_4, "__atomic_store_4", NULL,
+                       t, false, false);
+
+  t = build_function_type_list(void_type_node,
+                               ptr_type_node,
+                               uint64_type_node,
+                               integer_type_node,
+                               NULL_TREE);
+  this->define_builtin(BUILT_IN_ATOMIC_STORE_8, "__atomic_store_8", NULL,
+                       t, false, false);
+
+  t = build_function_type_list(uint32_type_node,
+                               ptr_type_node,
+                               uint32_type_node,
+                               integer_type_node,
+                               NULL_TREE);
+  this->define_builtin(BUILT_IN_ATOMIC_EXCHANGE_4, "__atomic_exchange_4", NULL,
+                       t, false, false);
+
+  t = build_function_type_list(uint64_type_node,
+                               ptr_type_node,
+                               uint64_type_node,
+                               integer_type_node,
+                               NULL_TREE);
+  this->define_builtin(BUILT_IN_ATOMIC_EXCHANGE_8, "__atomic_exchange_8", NULL,
+                       t, false, false);
+
+  t = build_function_type_list(boolean_type_node,
+                               ptr_type_node,
+                               ptr_type_node,
+                               uint32_type_node,
+                               boolean_type_node,
+                               integer_type_node,
+                               integer_type_node,
+                               NULL_TREE);
+  this->define_builtin(BUILT_IN_ATOMIC_COMPARE_EXCHANGE_4,
+                       "__atomic_compare_exchange_4", NULL,
+                       t, false, false);
+
+  t = build_function_type_list(boolean_type_node,
+                               ptr_type_node,
+                               ptr_type_node,
+                               uint64_type_node,
+                               boolean_type_node,
+                               integer_type_node,
+                               integer_type_node,
+                               NULL_TREE);
+  this->define_builtin(BUILT_IN_ATOMIC_COMPARE_EXCHANGE_8,
+                       "__atomic_compare_exchange_8", NULL,
+                       t, false, false);
+
+  t = build_function_type_list(uint32_type_node,
+                               ptr_type_node,
+                               uint32_type_node,
+                               integer_type_node,
+                               NULL_TREE);
+  this->define_builtin(BUILT_IN_ATOMIC_ADD_FETCH_4, "__atomic_add_fetch_4", NULL,
+                       t, false, false);
+
+  t = build_function_type_list(uint64_type_node,
+                               ptr_type_node,
+                               uint64_type_node,
+                               integer_type_node,
+                               NULL_TREE);
+  this->define_builtin(BUILT_IN_ATOMIC_ADD_FETCH_8, "__atomic_add_fetch_8", NULL,
+                       t, false, false);
+
+  t = build_function_type_list(unsigned_char_type_node,
+                               ptr_type_node,
+                               unsigned_char_type_node,
+                               integer_type_node,
+                               NULL_TREE);
+  this->define_builtin(BUILT_IN_ATOMIC_AND_FETCH_1, "__atomic_and_fetch_1", NULL,
+                       t, false, false);
+
+  t = build_function_type_list(unsigned_char_type_node,
+                               ptr_type_node,
+                               unsigned_char_type_node,
+                               integer_type_node,
+                               NULL_TREE);
+  this->define_builtin(BUILT_IN_ATOMIC_OR_FETCH_1, "__atomic_or_fetch_1", NULL,
+                       t, false, false);
 }
 
 // Get an unnamed integer type.
Index: gcc/go/gofrontend/MERGE
===================================================================
--- gcc/go/gofrontend/MERGE	(revision 271303)
+++ gcc/go/gofrontend/MERGE	(working copy)
@@ -1,4 +1,4 @@
-c0c8ad50627e3a59267e6e3de233a0b30cf64150
+f8a3668cbcfa3f8cd6c26c62bce416714cd401fc
 
 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 271303)
+++ gcc/go/gofrontend/expressions.cc	(working copy)
@@ -10460,9 +10460,16 @@ Call_expression::intrinsify(Gogo* gogo,
   Location loc = this->location();
 
   Type* int_type = Type::lookup_integer_type("int");
+  Type* int32_type = Type::lookup_integer_type("int32");
+  Type* int64_type = Type::lookup_integer_type("int64");
+  Type* uint_type = Type::lookup_integer_type("uint");
   Type* uint32_type = Type::lookup_integer_type("uint32");
   Type* uint64_type = Type::lookup_integer_type("uint64");
   Type* uintptr_type = Type::lookup_integer_type("uintptr");
+  Type* pointer_type = Type::make_pointer_type(Type::make_void_type());
+
+  int int_size = int_type->named_type()->real_type()->integer_type()->bits() / 8;
+  int ptr_size = uintptr_type->named_type()->real_type()->integer_type()->bits() / 8;
 
   if (package == "runtime")
     {
@@ -10545,6 +10552,242 @@ Call_expression::intrinsify(Gogo* gogo,
           return Expression::make_conditional(cmp, c64, call, loc);
         }
     }
+  else if (package == "runtime/internal/atomic")
+    {
+      int memorder = __ATOMIC_SEQ_CST;
+
+      if ((name == "Load" || name == "Load64" || name == "Loadint64" || name == "Loadp"
+           || name == "Loaduint" || name == "Loaduintptr" || name == "LoadAcq")
+          && this->args_ != NULL && this->args_->size() == 1)
+        {
+          if (int_size < 8 && (name == "Load64" || name == "Loadint64"))
+            // On 32-bit architectures we need to check alignment.
+            // Not intrinsify for now.
+            return NULL;
+
+          Runtime::Function code;
+          Type* res_type;
+          if (name == "Load")
+            {
+              code = Runtime::ATOMIC_LOAD_4;
+              res_type = uint32_type;
+            }
+          else if (name == "Load64")
+            {
+              code = Runtime::ATOMIC_LOAD_8;
+              res_type = uint64_type;
+            }
+          else if (name == "Loadint64")
+            {
+              code = Runtime::ATOMIC_LOAD_8;
+              res_type = int64_type;
+            }
+          else if (name == "Loaduint")
+            {
+              code = (int_size == 8
+                      ? Runtime::ATOMIC_LOAD_8
+                      : Runtime::ATOMIC_LOAD_4);
+              res_type = uint_type;
+            }
+          else if (name == "Loaduintptr")
+            {
+              code = (ptr_size == 8
+                      ? Runtime::ATOMIC_LOAD_8
+                      : Runtime::ATOMIC_LOAD_4);
+              res_type = uintptr_type;
+            }
+          else if (name == "Loadp")
+            {
+              code = (ptr_size == 8
+                      ? Runtime::ATOMIC_LOAD_8
+                      : Runtime::ATOMIC_LOAD_4);
+              res_type = pointer_type;
+            }
+          else if (name == "LoadAcq")
+            {
+              code = Runtime::ATOMIC_LOAD_4;
+              res_type = uint32_type;
+              memorder = __ATOMIC_ACQUIRE;
+            }
+          else
+            go_unreachable();
+          Expression* a1 = this->args_->front();
+          Expression* a2 = Expression::make_integer_ul(memorder, int32_type, loc);
+          Expression* call = Runtime::make_call(code, loc, 2, a1, a2);
+          return Expression::make_unsafe_cast(res_type, call, loc);
+        }
+
+      if ((name == "Store" || name == "Store64" || name == "StorepNoWB"
+           || name == "Storeuintptr" || name == "StoreRel")
+          && this->args_ != NULL && this->args_->size() == 2)
+        {
+          if (int_size < 8 && name == "Store64")
+            return NULL;
+
+          Runtime::Function code;
+          Expression* a1 = this->args_->at(0);
+          Expression* a2 = this->args_->at(1);
+          if (name == "Store")
+            code = Runtime::ATOMIC_STORE_4;
+          else if (name == "Store64")
+            code = Runtime::ATOMIC_STORE_8;
+          else if (name == "Storeuintptr")
+            code = (ptr_size == 8 ? Runtime::ATOMIC_STORE_8 : Runtime::ATOMIC_STORE_4);
+          else if (name == "StorepNoWB")
+            {
+              code = (ptr_size == 8 ? Runtime::ATOMIC_STORE_8 : Runtime::ATOMIC_STORE_4);
+              a2 = Expression::make_unsafe_cast(uintptr_type, a2, loc);
+              a2 = Expression::make_cast(uint64_type, a2, loc);
+            }
+          else if (name == "StoreRel")
+            {
+              code = Runtime::ATOMIC_STORE_4;
+              memorder = __ATOMIC_RELEASE;
+            }
+          else
+            go_unreachable();
+          Expression* a3 = Expression::make_integer_ul(memorder, int32_type, loc);
+          return Runtime::make_call(code, loc, 3, a1, a2, a3);
+        }
+
+      if ((name == "Xchg" || name == "Xchg64" || name == "Xchguintptr")
+          && this->args_ != NULL && this->args_->size() == 2)
+        {
+          if (int_size < 8 && name == "Xchg64")
+            return NULL;
+
+          Runtime::Function code;
+          Type* res_type;
+          if (name == "Xchg")
+            {
+              code = Runtime::ATOMIC_EXCHANGE_4;
+              res_type = uint32_type;
+            }
+          else if (name == "Xchg64")
+            {
+              code = Runtime::ATOMIC_EXCHANGE_8;
+              res_type = uint64_type;
+            }
+          else if (name == "Xchguintptr")
+            {
+              code = (ptr_size == 8
+                      ? Runtime::ATOMIC_EXCHANGE_8
+                      : Runtime::ATOMIC_EXCHANGE_4);
+              res_type = uintptr_type;
+            }
+          else
+            go_unreachable();
+          Expression* a1 = this->args_->at(0);
+          Expression* a2 = this->args_->at(1);
+          Expression* a3 = Expression::make_integer_ul(memorder, int32_type, loc);
+          Expression* call = Runtime::make_call(code, loc, 3, a1, a2, a3);
+          return Expression::make_cast(res_type, call, loc);
+        }
+
+      if ((name == "Cas" || name == "Cas64" || name == "Casuintptr"
+           || name == "Casp1" || name == "CasRel")
+          && this->args_ != NULL && this->args_->size() == 3)
+        {
+          if (int_size < 8 && name == "Cas64")
+            return NULL;
+
+          Runtime::Function code;
+          Expression* a1 = this->args_->at(0);
+
+          // Builtin cas takes a pointer to the old value.
+          // Store it in a temporary and take the address.
+          Expression* a2 = this->args_->at(1);
+          Temporary_statement* ts = Statement::make_temporary(NULL, a2, loc);
+          inserter->insert(ts);
+          a2 = Expression::make_temporary_reference(ts, loc);
+          a2 = Expression::make_unary(OPERATOR_AND, a2, loc);
+
+          Expression* a3 = this->args_->at(2);
+          if (name == "Cas")
+            code = Runtime::ATOMIC_COMPARE_EXCHANGE_4;
+          else if (name == "Cas64")
+            code = Runtime::ATOMIC_COMPARE_EXCHANGE_8;
+          else if (name == "Casuintptr")
+            code = (ptr_size == 8
+                    ? Runtime::ATOMIC_COMPARE_EXCHANGE_8
+                    : Runtime::ATOMIC_COMPARE_EXCHANGE_4);
+          else if (name == "Casp1")
+            {
+              code = (ptr_size == 8
+                      ? Runtime::ATOMIC_COMPARE_EXCHANGE_8
+                      : Runtime::ATOMIC_COMPARE_EXCHANGE_4);
+              a3 = Expression::make_unsafe_cast(uintptr_type, a3, loc);
+              a3 = Expression::make_cast(uint64_type, a3, loc);
+            }
+          else if (name == "CasRel")
+            {
+              code = Runtime::ATOMIC_COMPARE_EXCHANGE_4;
+              memorder = __ATOMIC_RELEASE;
+            }
+          else
+            go_unreachable();
+          Expression* a4 = Expression::make_boolean(false, loc);
+          Expression* a5 = Expression::make_integer_ul(memorder, int32_type, loc);
+          Expression* a6 = Expression::make_integer_ul(__ATOMIC_RELAXED, int32_type, loc);
+          return Runtime::make_call(code, loc, 6, a1, a2, a3, a4, a5, a6);
+        }
+
+      if ((name == "Xadd" || name == "Xadd64" || name == "Xaddint64"
+           || name == "Xadduintptr")
+          && this->args_ != NULL && this->args_->size() == 2)
+        {
+          if (int_size < 8 && (name == "Xadd64" || name == "Xaddint64"))
+            return NULL;
+
+          Runtime::Function code;
+          Type* res_type;
+          if (name == "Xadd")
+            {
+              code = Runtime::ATOMIC_ADD_FETCH_4;
+              res_type = uint32_type;
+            }
+          else if (name == "Xadd64")
+            {
+              code = Runtime::ATOMIC_ADD_FETCH_8;
+              res_type = uint64_type;
+            }
+          else if (name == "Xaddint64")
+            {
+              code = Runtime::ATOMIC_ADD_FETCH_8;
+              res_type = int64_type;
+            }
+          else if (name == "Xadduintptr")
+            {
+              code = (ptr_size == 8
+                      ? Runtime::ATOMIC_ADD_FETCH_8
+                      : Runtime::ATOMIC_ADD_FETCH_4);
+              res_type = uintptr_type;
+            }
+          else
+            go_unreachable();
+          Expression* a1 = this->args_->at(0);
+          Expression* a2 = this->args_->at(1);
+          Expression* a3 = Expression::make_integer_ul(memorder, int32_type, loc);
+          Expression* call = Runtime::make_call(code, loc, 3, a1, a2, a3);
+          return Expression::make_cast(res_type, call, loc);
+        }
+
+      if ((name == "And8" || name == "Or8")
+          && this->args_ != NULL && this->args_->size() == 2)
+        {
+          Runtime::Function code;
+          if (name == "And8")
+            code = Runtime::ATOMIC_AND_FETCH_1;
+          else if (name == "Or8")
+            code = Runtime::ATOMIC_OR_FETCH_1;
+          else
+            go_unreachable();
+          Expression* a1 = this->args_->at(0);
+          Expression* a2 = this->args_->at(1);
+          Expression* a3 = Expression::make_integer_ul(memorder, int32_type, loc);
+          return Runtime::make_call(code, loc, 3, a1, a2, a3);
+        }
+    }
 
   return NULL;
 }
Index: gcc/go/gofrontend/runtime.cc
===================================================================
--- gcc/go/gofrontend/runtime.cc	(revision 271303)
+++ gcc/go/gofrontend/runtime.cc	(working copy)
@@ -30,6 +30,8 @@ enum Runtime_function_type
   RFT_BOOLPTR,
   // Go type int, C type intgo.
   RFT_INT,
+  // Go type uint8, C type uint8_t.
+  RFT_UINT8,
   // Go type int32, C type int32_t.
   RFT_INT32,
   // Go type uint32, C type uint32_t.
@@ -109,6 +111,10 @@ runtime_function_type(Runtime_function_t
 	  t = Type::lookup_integer_type("int");
 	  break;
 
+	case RFT_UINT8:
+	  t = Type::lookup_integer_type("uint8");
+	  break;
+
 	case RFT_INT32:
 	  t = Type::lookup_integer_type("int32");
 	  break;
@@ -250,6 +256,7 @@ convert_to_runtime_function_type(Runtime
     case RFT_BOOL:
     case RFT_BOOLPTR:
     case RFT_INT:
+    case RFT_UINT8:
     case RFT_INT32:
     case RFT_UINT32:
     case RFT_INT64:
Index: gcc/go/gofrontend/runtime.def
===================================================================
--- gcc/go/gofrontend/runtime.def	(revision 271303)
+++ gcc/go/gofrontend/runtime.def	(working copy)
@@ -396,6 +396,38 @@ DEF_GO_RUNTIME(BUILTIN_BSWAP64, "__built
 DEF_GO_RUNTIME(BUILTIN_CTZ, "__builtin_ctz", P1(UINT32), R1(INT32))
 DEF_GO_RUNTIME(BUILTIN_CTZLL, "__builtin_ctzll", P1(UINT64), R1(INT32))
 
+// Atomics.
+DEF_GO_RUNTIME(ATOMIC_LOAD_4, "__atomic_load_4", P2(POINTER, INT32),
+               R1(UINT32))
+DEF_GO_RUNTIME(ATOMIC_LOAD_8, "__atomic_load_8", P2(POINTER, INT32),
+               R1(UINT64))
+DEF_GO_RUNTIME(ATOMIC_STORE_4, "__atomic_store_4", P3(POINTER, UINT32, INT32),
+               R0())
+DEF_GO_RUNTIME(ATOMIC_STORE_8, "__atomic_store_8", P3(POINTER, UINT64, INT32),
+               R0())
+DEF_GO_RUNTIME(ATOMIC_EXCHANGE_4, "__atomic_exchange_4", P3(POINTER, UINT32, INT32),
+               R1(UINT32))
+DEF_GO_RUNTIME(ATOMIC_EXCHANGE_8, "__atomic_exchange_8", P3(POINTER, UINT64, INT32),
+               R1(UINT64))
+DEF_GO_RUNTIME(ATOMIC_COMPARE_EXCHANGE_4, "__atomic_compare_exchange_4",
+               P6(POINTER, POINTER, UINT32, BOOL, INT32, INT32),
+               R1(BOOL))
+DEF_GO_RUNTIME(ATOMIC_COMPARE_EXCHANGE_8, "__atomic_compare_exchange_8",
+               P6(POINTER, POINTER, UINT64, BOOL, INT32, INT32),
+               R1(BOOL))
+DEF_GO_RUNTIME(ATOMIC_ADD_FETCH_4, "__atomic_add_fetch_4",
+               P3(POINTER, UINT32, INT32),
+               R1(UINT32))
+DEF_GO_RUNTIME(ATOMIC_ADD_FETCH_8, "__atomic_add_fetch_8",
+               P3(POINTER, UINT64, INT32),
+               R1(UINT64))
+DEF_GO_RUNTIME(ATOMIC_AND_FETCH_1, "__atomic_and_fetch_1",
+               P3(POINTER, UINT8, INT32),
+               R1(UINT8))
+DEF_GO_RUNTIME(ATOMIC_OR_FETCH_1, "__atomic_or_fetch_1",
+               P3(POINTER, UINT8, INT32),
+               R1(UINT8))
+
 // Remove helper macros.
 #undef ABFT6
 #undef ABFT2

Re: Go patch committed: Intrinsify runtime/internal/atomic functions

[Andreas Schwab]

I'm getting this crash on riscv:

/daten/riscv64/gcc/gcc-20190518/Build/./gcc/gccgo -B/daten/riscv64/gcc/gcc-20190518/Build/./gcc/ -B/usr/riscv64-suse-linux/bin/ -B/usr/riscv64-suse-linux/lib/ -isystem /usr/riscv64-suse-linux/include -isystem /usr/riscv64-suse-linux/sys-include -O2 -g -I . -c -fgo-pkgpath=runtime -fgo-c-header=runtime.inc.raw -fgo-compiling-runtime ../../../libgo/go/runtime/alg.go ../../../libgo/go/runtime/atomic_pointer.go ../../../libgo/go/runtime/cgo_gccgo.go ../../../libgo/go/runtime/cgocall.go ../../../libgo/go/runtime/cgocheck.go ../../../libgo/go/runtime/chan.go ../../../libgo/go/runtime/compiler.go ../../../libgo/go/runtime/cpuprof.go ../../../libgo/go/runtime/cputicks.go ../../../libgo/go/runtime/debug.go ../../../libgo/go/runtime/env_posix.go ../../../libgo/go/runtime/error.go ../../../libgo/go/runtime/extern.go ../../../libgo/go/runtime/fastlog2.go ../../../libgo/go/runtime/fastlog2table.go ../../../libgo/go/runtime/ffi.go ../../../libgo/go/runtime/float.go ../../../libgo/go/runtime/hash64.go ../../../libgo/go/runtime/heapdump.go ../../../libgo/go/runtime/iface.go ../../../libgo/go/runtime/lfstack.go ../../../libgo/go/runtime/lfstack_64bit.go ../../../libgo/go/runtime/lock_futex.go ../../../libgo/go/runtime/malloc.go ../../../libgo/go/runtime/map.go ../../../libgo/go/runtime/map_fast32.go ../../../libgo/go/runtime/map_fast64.go ../../../libgo/go/runtime/map_faststr.go ../../../libgo/go/runtime/mbarrier.go ../../../libgo/go/runtime/mbitmap.go ../../../libgo/go/runtime/mcache.go ../../../libgo/go/runtime/mcentral.go ../../../libgo/go/runtime/mem_gccgo.go ../../../libgo/go/runtime/mfinal.go ../../../libgo/go/runtime/mfixalloc.go ../../../libgo/go/runtime/mgc.go ../../../libgo/go/runtime/mgc_gccgo.go ../../../libgo/go/runtime/mgclarge.go ../../../libgo/go/runtime/mgcmark.go ../../../libgo/go/runtime/mgcsweep.go ../../../libgo/go/runtime/mgcsweepbuf.go ../../../libgo/go/runtime/mgcwork.go ../../../libgo/go/runtime/mheap.go ../../../libgo/go/runtime/mprof.go ../../../libgo/go/runtime/msan0.go ../../../libgo/go/runtime/msize.go ../../../libgo/go/runtime/mstats.go ../../../libgo/go/runtime/mwbbuf.go ../../../libgo/go/runtime/netpoll.go ../../../libgo/go/runtime/netpoll_epoll.go ../../../libgo/go/runtime/os_gccgo.go ../../../libgo/go/runtime/os_linux.go ../../../libgo/go/runtime/os_linux_noauxv.go ../../../libgo/go/runtime/panic.go ../../../libgo/go/runtime/print.go ../../../libgo/go/runtime/proc.go ../../../libgo/go/runtime/profbuf.go ../../../libgo/go/runtime/proflabel.go ../../../libgo/go/runtime/race0.go ../../../libgo/go/runtime/rdebug.go ../../../libgo/go/runtime/relax_stub.go ../../../libgo/go/runtime/runtime.go ../../../libgo/go/runtime/runtime1.go ../../../libgo/go/runtime/runtime2.go ../../../libgo/go/runtime/rwmutex.go ../../../libgo/go/runtime/select.go ../../../libgo/go/runtime/sema.go ../../../libgo/go/runtime/signal_gccgo.go ../../../libgo/go/runtime/signal_sighandler.go ../../../libgo/go/runtime/signal_unix.go ../../../libgo/go/runtime/sigqueue.go ../../../libgo/go/runtime/sizeclasses.go ../../../libgo/go/runtime/slice.go ../../../libgo/go/runtime/string.go ../../../libgo/go/runtime/stubs.go ../../../libgo/go/runtime/stubs2.go ../../../libgo/go/runtime/stubs3.go ../../../libgo/go/runtime/stubs_linux.go ../../../libgo/go/runtime/symtab.go ../../../libgo/go/runtime/time.go ../../../libgo/go/runtime/timestub.go ../../../libgo/go/runtime/timestub2.go ../../../libgo/go/runtime/trace.go ../../../libgo/go/runtime/traceback_gccgo.go ../../../libgo/go/runtime/type.go ../../../libgo/go/runtime/typekind.go ../../../libgo/go/runtime/unaligned1.go ../../../libgo/go/runtime/utf8.go ../../../libgo/go/runtime/write_err.go runtime_sysinfo.go sigtab.go  -fPIC -o .libs/runtime.o
during RTL pass: expand
../../../libgo/go/runtime/mbitmap.go: In function ‘runtime.setMarked.runtime.markBits’:
../../../libgo/go/runtime/mbitmap.go:291:9: internal compiler error: Segmentation fault
  291 |  atomic.Or8(m.bytep, m.mask)
      |         ^
0x6a02b7 crash_signal
        ../../gcc/toplev.c:326
0x917cf6 get_callee_fndecl(tree_node const*)
        ../../gcc/tree.c:9570
0x2c2e6b expand_call(tree_node*, rtx_def*, int)
        ../../gcc/calls.c:3364
0x2aa3e9 expand_builtin_atomic_fetch_op
        ../../gcc/builtins.c:6541
0x2b5981 expand_builtin(tree_node*, rtx_def*, rtx_def*, machine_mode, int)
        ../../gcc/builtins.c:8220
0x3bdfef expand_expr_real_1(tree_node*, rtx_def*, machine_mode, expand_modifier, rtx_def**, bool)
        ../../gcc/expr.c:11030
0x2d4ee5 expand_expr
        ../../gcc/expr.h:279
0x2d4ee5 expand_call_stmt
        ../../gcc/cfgexpand.c:2724
0x2d4ee5 expand_gimple_stmt_1
        ../../gcc/cfgexpand.c:3700
0x2d5847 expand_gimple_stmt
        ../../gcc/cfgexpand.c:3859
0x2da083 expand_gimple_basic_block
        ../../gcc/cfgexpand.c:5895
0x2dbff3 execute
        ../../gcc/cfgexpand.c:6518

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: Go patch committed: Intrinsify runtime/internal/atomic functions

[Jim Wilson]

[-- Attachment #1: Type: text/plain, Size: 1560 bytes --]

On Sun, May 19, 2019 at 5:22 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
> ../../../libgo/go/runtime/mbitmap.go: In function ‘runtime.setMarked.runtime.markBits’:
> ../../../libgo/go/runtime/mbitmap.go:291:9: internal compiler error: Segmentation fault
>   291 |  atomic.Or8(m.bytep, m.mask)
>       |         ^

This is failing for RISC-V because __atomic_or_fetch_1 isn't a
built-in function that can be expanded inline.  You have to call the
library function in libatomic.  The C front-end is registering all of
the built-in functions, but it looks like the go front-end is only
registering functions it thinks it needs and this list is incomplete.
In expand_builtin, case BUILT_IN_ATOMIC_OR_FETCH_1, the external
library call for this gets set to BUILT_IN_ATOMIC_FETCH_OR_1.  Then in
expand_builtin_atomic_fetch_op when we call builtin_decl_explicit
(ext_call) it returns NULL.  This is because the go front end
registered BUILT_IN_ATOMIC_OR_FETCH_1 as a built-in, but did not
register BUILT_IN_ATOMIC_FETCH_OR_1 as a built-in.  The NULL return
from builtin_decl_explicit gives us an ADDR_EXPR with a NULL operand
which eventually causes the internal compiler error.  It looks like
the same thing is done with all of the op_fetch built-ins, so use of
any of them means that the fetch_op built-in also has to be
registered.  I verified with a quick hack that I need both
BUILT_IN_ATOMIC_FETCH_OR_1 and BUILT_IN_ATOMIC_FETCH_AND_1 defined as
built-ins to make a RISC-V go build work.  I haven't done any testing
yet.

Jim

[-- Attachment #2: go-builtins.txt --]
[-- Type: text/plain, Size: 955 bytes --]

diff --git a/gcc/go/go-gcc.cc b/gcc/go/go-gcc.cc
index 1b26f2bac93..91043b51463 100644
--- a/gcc/go/go-gcc.cc
+++ b/gcc/go/go-gcc.cc
@@ -871,6 +871,8 @@ Gcc_backend::Gcc_backend()
                                NULL_TREE);
   this->define_builtin(BUILT_IN_ATOMIC_AND_FETCH_1, "__atomic_and_fetch_1", NULL,
                        t, false, false);
+  this->define_builtin(BUILT_IN_ATOMIC_FETCH_AND_1, "__atomic_fetch_and_1", NULL,
+                       t, false, false);
 
   t = build_function_type_list(unsigned_char_type_node,
                                ptr_type_node,
@@ -879,6 +881,8 @@ Gcc_backend::Gcc_backend()
                                NULL_TREE);
   this->define_builtin(BUILT_IN_ATOMIC_OR_FETCH_1, "__atomic_or_fetch_1", NULL,
                        t, false, false);
+  this->define_builtin(BUILT_IN_ATOMIC_FETCH_OR_1, "__atomic_fetch_or_1", NULL,
+                       t, false, false);
 }
 
 // Get an unnamed integer type.

Re: [gofrontend-dev] Re: Go patch committed: Intrinsify runtime/internal/atomic functions

[Cherry Zhang via gcc-patches]

Jim, thank you for the fix! The patch looks good to me. Maybe we should
also apply this to __atomic_add_fetch_4 and __atomic_add_fetch_8?

Thanks,
Cherry



On Tue, May 21, 2019 at 11:25 PM Jim Wilson <jimw@sifive.com> wrote:

> On Sun, May 19, 2019 at 5:22 AM Andreas Schwab <schwab@linux-m68k.org>
> wrote:
> > ../../../libgo/go/runtime/mbitmap.go: In function
> ‘runtime.setMarked.runtime.markBits’:
> > ../../../libgo/go/runtime/mbitmap.go:291:9: internal compiler error:
> Segmentation fault
> >   291 |  atomic.Or8(m.bytep, m.mask)
> >       |         ^
>
> This is failing for RISC-V because __atomic_or_fetch_1 isn't a
> built-in function that can be expanded inline.  You have to call the
> library function in libatomic.  The C front-end is registering all of
> the built-in functions, but it looks like the go front-end is only
> registering functions it thinks it needs and this list is incomplete.
> In expand_builtin, case BUILT_IN_ATOMIC_OR_FETCH_1, the external
> library call for this gets set to BUILT_IN_ATOMIC_FETCH_OR_1.  Then in
> expand_builtin_atomic_fetch_op when we call builtin_decl_explicit
> (ext_call) it returns NULL.  This is because the go front end
> registered BUILT_IN_ATOMIC_OR_FETCH_1 as a built-in, but did not
> register BUILT_IN_ATOMIC_FETCH_OR_1 as a built-in.  The NULL return
> from builtin_decl_explicit gives us an ADDR_EXPR with a NULL operand
> which eventually causes the internal compiler error.  It looks like
> the same thing is done with all of the op_fetch built-ins, so use of
> any of them means that the fetch_op built-in also has to be
> registered.  I verified with a quick hack that I need both
> BUILT_IN_ATOMIC_FETCH_OR_1 and BUILT_IN_ATOMIC_FETCH_AND_1 defined as
> built-ins to make a RISC-V go build work.  I haven't done any testing
> yet.
>
> Jim
>
> --
> You received this message because you are subscribed to the Google Groups
> "gofrontend-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to gofrontend-dev+unsubscribe@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/gofrontend-dev/CAFyWVaY8aMmhmnWpoGdywaeEhiXmSUe8qqha%2BHhvbxYHhUnisQ%40mail.gmail.com
> .
> For more options, visit https://groups.google.com/d/optout.
>

Re: [gofrontend-dev] Re: Go patch committed: Intrinsify runtime/internal/atomic functions

[Ian Lance Taylor]

On Wed, May 22, 2019 at 9:36 PM Cherry Zhang <cherryyz@google.com> wrote:
>
> Jim, thank you for the fix! The patch looks good to me. Maybe we should also apply this to __atomic_add_fetch_4 and __atomic_add_fetch_8?
>
> Thanks,
> Cherry
>
>
>
> On Tue, May 21, 2019 at 11:25 PM Jim Wilson <jimw@sifive.com> wrote:
>>
>> On Sun, May 19, 2019 at 5:22 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
>> > ../../../libgo/go/runtime/mbitmap.go: In function ‘runtime.setMarked.runtime.markBits’:
>> > ../../../libgo/go/runtime/mbitmap.go:291:9: internal compiler error: Segmentation fault
>> >   291 |  atomic.Or8(m.bytep, m.mask)
>> >       |         ^
>>
>> This is failing for RISC-V because __atomic_or_fetch_1 isn't a
>> built-in function that can be expanded inline.  You have to call the
>> library function in libatomic.  The C front-end is registering all of
>> the built-in functions, but it looks like the go front-end is only
>> registering functions it thinks it needs and this list is incomplete.
>> In expand_builtin, case BUILT_IN_ATOMIC_OR_FETCH_1, the external
>> library call for this gets set to BUILT_IN_ATOMIC_FETCH_OR_1.  Then in
>> expand_builtin_atomic_fetch_op when we call builtin_decl_explicit
>> (ext_call) it returns NULL.  This is because the go front end
>> registered BUILT_IN_ATOMIC_OR_FETCH_1 as a built-in, but did not
>> register BUILT_IN_ATOMIC_FETCH_OR_1 as a built-in.  The NULL return
>> from builtin_decl_explicit gives us an ADDR_EXPR with a NULL operand
>> which eventually causes the internal compiler error.  It looks like
>> the same thing is done with all of the op_fetch built-ins, so use of
>> any of them means that the fetch_op built-in also has to be
>> registered.  I verified with a quick hack that I need both
>> BUILT_IN_ATOMIC_FETCH_OR_1 and BUILT_IN_ATOMIC_FETCH_AND_1 defined as
>> built-ins to make a RISC-V go build work.  I haven't done any testing
>> yet.

Jim, you can go ahead and commit that patch with a ChangeLog entry.
(The files in go/gcc but not in go/gcc/gofrontend) are under normal
GCC rules.)  Thanks.

Ian

Re: Go patch committed: Intrinsify runtime/internal/atomic functions

[Andreas Schwab]

On Mai 21 2019, Jim Wilson <jimw@sifive.com> wrote:

> On Sun, May 19, 2019 at 5:22 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
>> ../../../libgo/go/runtime/mbitmap.go: In function ‘runtime.setMarked.runtime.markBits’:
>> ../../../libgo/go/runtime/mbitmap.go:291:9: internal compiler error: Segmentation fault
>>   291 |  atomic.Or8(m.bytep, m.mask)
>>       |         ^
>
> This is failing for RISC-V because __atomic_or_fetch_1 isn't a
> built-in function that can be expanded inline.  You have to call the
> library function in libatomic.  The C front-end is registering all of
> the built-in functions, but it looks like the go front-end is only
> registering functions it thinks it needs and this list is incomplete.
> In expand_builtin, case BUILT_IN_ATOMIC_OR_FETCH_1, the external
> library call for this gets set to BUILT_IN_ATOMIC_FETCH_OR_1.  Then in
> expand_builtin_atomic_fetch_op when we call builtin_decl_explicit
> (ext_call) it returns NULL.  This is because the go front end
> registered BUILT_IN_ATOMIC_OR_FETCH_1 as a built-in, but did not
> register BUILT_IN_ATOMIC_FETCH_OR_1 as a built-in.  The NULL return
> from builtin_decl_explicit gives us an ADDR_EXPR with a NULL operand
> which eventually causes the internal compiler error.  It looks like
> the same thing is done with all of the op_fetch built-ins, so use of
> any of them means that the fetch_op built-in also has to be
> registered.  I verified with a quick hack that I need both
> BUILT_IN_ATOMIC_FETCH_OR_1 and BUILT_IN_ATOMIC_FETCH_AND_1 defined as
> built-ins to make a RISC-V go build work.  I haven't done any testing
> yet.

Here are the test results:
http://gcc.gnu.org/ml/gcc-testresults/2019-05/msg02903.html

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: [gofrontend-dev] Re: Go patch committed: Intrinsify runtime/internal/atomic functions

[Jim Wilson]

On Wed, May 22, 2019 at 6:41 PM Ian Lance Taylor <iant@golang.org> wrote:
> Jim, you can go ahead and commit that patch with a ChangeLog entry.
> (The files in go/gcc but not in go/gcc/gofrontend) are under normal
> GCC rules.)  Thanks.

OK.  Committed.  I did an x86_64-linux go build and check to make sure
I didn't break that.

Jim

Re: [gofrontend-dev] Re: Go patch committed: Intrinsify runtime/internal/atomic functions

[Jim Wilson]

On Wed, May 22, 2019 at 6:36 PM Cherry Zhang <cherryyz@google.com> wrote:
> Jim, thank you for the fix! The patch looks good to me. Maybe we should also apply this to __atomic_add_fetch_4 and __atomic_add_fetch_8?

Sorry about the delay, I caught a virus last week and am behind on everything.

For 64-bit RISC-V those are open coded and hence not a problems.  All
of the major targets open code everything except RISC-V which needs a
library call for the 1 and 2 byte atomics, though this is on our list
of things to fix.  We just haven't gotten around to it yet.  For
32-bit RISC-V, the 8-byte atomic would require a library call, but
32-bit riscv-linux isn't formally supported, as the glibc patches are
not upstream yet, so this isn't a practical problem yet.

Anyways, yes, in theory, we should be handling these two also, but
there probably aren't any well supported targets that will fail if
they are missing.

Jim

Re: Go patch committed: Intrinsify runtime/internal/atomic functions

[Jim Wilson]

On Thu, May 30, 2019 at 11:11 AM Andreas Schwab <schwab@linux-m68k.org> wrote:
> Here are the test results:
> http://gcc.gnu.org/ml/gcc-testresults/2019-05/msg02903.html

I tried running RISC-V go checks on my system.  I see 7 unexpected
failures for gcc check-go, 6 unexpected failures for check-gotools,
and 1 unexpected failure for check-target-libgo.  I haven't tried
building and testing the go support before so I don't have any old
results to compare against.  It seems reasonable for a first attempt
though.

Jim

Re: Go patch committed: Intrinsify runtime/internal/atomic functions

[Maciej Rozycki]

On Thu, 30 May 2019, Jim Wilson wrote:

> > Here are the test results:
> > http://gcc.gnu.org/ml/gcc-testresults/2019-05/msg02903.html
> 
> I tried running RISC-V go checks on my system.  I see 7 unexpected
> failures for gcc check-go, 6 unexpected failures for check-gotools,
> and 1 unexpected failure for check-target-libgo.  I haven't tried
> building and testing the go support before so I don't have any old
> results to compare against.  It seems reasonable for a first attempt
> though.

 I have results as at r270765, taken with QEMU run in the user emulation 
mode (I have patches outstanding for configury/Makefiles to correctly 
support libgo and other library testing with a build sysroot in such a 
setup, pending the completion of my WDC copyright assignment paperwork 
with FSF):

FAIL: go.test/test/args.go execution,  -O2 -g
FAIL: go.test/test/fixedbugs/bug347.go execution,  -O0 -g
FAIL: go.test/test/fixedbugs/bug348.go execution,  -O0 -g
FAIL: go.test/test/fixedbugs/issue4562.go execution,  -O2 -g
FAIL: go.test/test/fixedbugs/issue6055.go execution,  -O2 -g
FAIL: go.test/test/method5.go execution,  -O2 -g
FAIL: go.test/test/nil.go execution,  -O2 -g
FAIL: go.test/test/recover3.go execution,  -O2 -g

		=== go Summary ===

# of expected passes		3188
# of unexpected failures	8
# of expected failures		1
# of untested testcases		32
# of unsupported tests		3

I don't know why the number of passes (effectively # of tests run) is 
reduced by more than a half with otherwise similar figures; it could be 
due to the use of a simulator vs native test setup.

 For some reason I haven't looked into libgo tests are deliberatly run 
individually and results, concatenated into libgo-all.sum, have to be 
processed manually to get a summary.  Here's one I have come up with:

WARNING: program timed out.
FAIL: archive/zip
FAIL: cmd/go/internal/lockedfile/internal/filelock
FAIL: compress/flate
FAIL: compress/lzw
FAIL: compress/zlib
WARNING: program timed out.
FAIL: crypto/dsa
FAIL: crypto/tls
FAIL: database/sql
FAIL: html/template
FAIL: image/draw
FAIL: image/jpeg
FAIL: internal/cpu
FAIL: internal/x/crypto/cryptobyte
FAIL: internal/x/net/http/httpproxy
FAIL: log/syslog
FAIL: net
FAIL: net/http
FAIL: net/http/cgi
FAIL: net/http/internal
FAIL: net/http/pprof
FAIL: os
FAIL: os/exec
FAIL: os/signal
FAIL: reflect
WARNING: program timed out.
FAIL: regexp
WARNING: program timed out.
FAIL: runtime
FAIL: runtime/pprof
FAIL: sync
FAIL: sync/atomic
FAIL: syscall
FAIL: text/template
FAIL: time

		=== libgo Summary ===

# of expected passes		136
# of unexpected failures	32

These are significantly worse, but I suspect the use of the user emulation 
mode of QEMU may have contributed here.

 No gotools tests run in my setup for some reason:

make[2]: Entering directory '.../gcc/gotools'
make[2]: Nothing to be done for 'check'.
make[2]: Leaving directory '.../gcc/gotools'

-- maybe this is not supported with a cross-toolchain.

 I have current test results (also for the other frontends/libraries, e.g. 
GNAT) with the same setup as well in case someone is interested.  There 
are minor differences.

  Maciej

Re: Go patch committed: Intrinsify runtime/internal/atomic functions

[Ian Lance Taylor]

On Mon, Jun 3, 2019 at 7:12 AM Maciej Rozycki <macro@wdc.com> wrote:
>
> On Thu, 30 May 2019, Jim Wilson wrote:
>
> > > Here are the test results:
> > > http://gcc.gnu.org/ml/gcc-testresults/2019-05/msg02903.html
> >
> > I tried running RISC-V go checks on my system.  I see 7 unexpected
> > failures for gcc check-go, 6 unexpected failures for check-gotools,
> > and 1 unexpected failure for check-target-libgo.  I haven't tried
> > building and testing the go support before so I don't have any old
> > results to compare against.  It seems reasonable for a first attempt
> > though.
>
>  I have results as at r270765, taken with QEMU run in the user emulation
> mode (I have patches outstanding for configury/Makefiles to correctly
> support libgo and other library testing with a build sysroot in such a
> setup, pending the completion of my WDC copyright assignment paperwork
> with FSF):
>
> FAIL: go.test/test/args.go execution,  -O2 -g

That is a fairly trivial test so if that one fails then something is
pretty fundamentally wrong somewhere.

You can run just that test by running

make RUNTESTFLAGS=go-test.exp=args.go check-gcc-go

If it fails you should have an executable gcc/testsuite/go/args.x.
Run that executable as `args.x arg1 arg2`.

Ian

Re: Go patch committed: Intrinsify runtime/internal/atomic functions

[Andreas Schwab]

On Jun 03 2019, Maciej Rozycki <macro@wdc.com> wrote:

> These are significantly worse, but I suspect the use of the user emulation 
> mode of QEMU may have contributed here.

I think all your failures are due to bugs/limitations in the qemu
emulation.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510  2552 DF73 E780 A9DA AEC1
"And now for something completely different."

Re: Go patch committed: Intrinsify runtime/internal/atomic functions

[Maciej Rozycki]

On Mon, 3 Jun 2019, Ian Lance Taylor wrote:

> >  I have results as at r270765, taken with QEMU run in the user emulation
> > mode (I have patches outstanding for configury/Makefiles to correctly
> > support libgo and other library testing with a build sysroot in such a
> > setup, pending the completion of my WDC copyright assignment paperwork
> > with FSF):
> >
> > FAIL: go.test/test/args.go execution,  -O2 -g
> 
> That is a fairly trivial test so if that one fails then something is
> pretty fundamentally wrong somewhere.
> 
> You can run just that test by running
> 
> make RUNTESTFLAGS=go-test.exp=args.go check-gcc-go
> 
> If it fails you should have an executable gcc/testsuite/go/args.x.

 The .log file reports (leading paths edited throughout):

spawn qemu-riscv64 -E LD_LIBRARY_PATH=.:.../riscv64-linux-gnu/lib64/lp64d/libgo/.libs:.../gcc:.../gcc/lib64/lp64:.../gcc/lib64/lp64d .../gcc/testsuite/go/args.x
panic: argc

goroutine 1 [running]:
main.main
	.../gcc/testsuite/go.test/test/args.go:18
FAIL: go.test/test/args.go execution,  -O2 -g

The same happens when this is invoked manually.

> Run that executable as `args.x arg1 arg2`.

 That works however.  I've modified `sim_spawn', which is declared as:

proc sim_spawn { dest cmdline args }

by adding this line:

clone_output "sim_spawn $dest $cmdline $args"

and it reports no command-line arguments being passed to the test case 
from the test harness:

sim_spawn qemu-user-lp64d .../gcc/testsuite/go/args.x

so I gather there is something wrong at an upper level.  I'll see if I can 
investigate it at one point, but given that it is probably the only test 
case that expects arguments I consider it a very low priority issue.  
Also not all simulators may support arguments in the first place, 
especially for bare metal.

 NB I had to make a local copy of `sim_spawn' in the qemu-user-lp64d.exp 
board description file used, to get `-E ...' passed to QEMU with the 
LD_LIBRARY_PATH setting for uninstalled shared libraries to work.  That 
does not affect received `args' being empty however.

  Maciej