Merge #1622

1622: intrinsics: Add atomic_load_* r=CohenArthur a=CohenArthur



Co-authored-by: Arthur Cohen <arthur.cohen@embecosm.com>

5 files changed, 172 insertions, 55 deletions

diff --git a/gcc/rust/backend/rust-builtins.cc b/gcc/rust/backend/rust-builtins.cc
index 64e06e1..66b3bec 100644
--- a/gcc/rust/backend/rust-builtins.cc
+++ b/gcc/rust/backend/rust-builtins.cc
@@ -71,36 +71,42 @@ BuiltinsContext::setup_math_fns ()
 void
 BuiltinsContext::setup_atomic_fns ()
 {
-  define_builtin ("atomic_store", BUILT_IN_ATOMIC_STORE, "__atomic_store", NULL,
-		  build_function_type_list (void_type_node, size_type_node,
-					    build_pointer_type (void_type_node),
-					    const_ptr_type_node,
-					    integer_type_node, NULL_TREE),
-		  0);
-  define_builtin ("atomic_store_n", BUILT_IN_ATOMIC_STORE_N, "__atomic_store_n",
-		  NULL,
-		  build_varargs_function_type_list (void_type_node, NULL_TREE),
-		  0);
+  auto atomic_store_type
+    = build_varargs_function_type_list (void_type_node, NULL_TREE);
+  auto atomic_load_type = [] (tree ret_type_node) {
+    return build_function_type_list (ret_type_node,
+				     ptr_type_node, // const_ptr_type_node?
+				     integer_type_node, NULL_TREE);
+  };
+
+  // FIXME: These should be the definition for the generic version of the
+  // atomic_store builtins, but I cannot get them to work properly. Revisit
+  // later. define_builtin ("atomic_store", BUILT_IN_ATOMIC_STORE,
+  // "__atomic_store", NULL,
+  //   atomic_store_type, 0);
+  // define_builtin ("atomic_store_n", BUILT_IN_ATOMIC_STORE_N,
+  // "__atomic_store_n",
+  //   NULL, atomic_store_type, 0);
+
   define_builtin ("atomic_store_1", BUILT_IN_ATOMIC_STORE_1, "__atomic_store_1",
-		  NULL,
-		  build_varargs_function_type_list (void_type_node, NULL_TREE),
-		  0);
+		  NULL, atomic_store_type, 0);
   define_builtin ("atomic_store_2", BUILT_IN_ATOMIC_STORE_2, "__atomic_store_2",
-		  NULL,
-		  build_varargs_function_type_list (void_type_node, NULL_TREE),
-		  0);
+		  NULL, atomic_store_type, 0);
   define_builtin ("atomic_store_4", BUILT_IN_ATOMIC_STORE_4, "__atomic_store_4",
-		  NULL,
-		  build_varargs_function_type_list (void_type_node, NULL_TREE),
-		  0);
+		  NULL, atomic_store_type, 0);
   define_builtin ("atomic_store_8", BUILT_IN_ATOMIC_STORE_8, "__atomic_store_8",
-		  NULL,
-		  build_varargs_function_type_list (void_type_node, NULL_TREE),
-		  0);
+		  NULL, atomic_store_type, 0);
   define_builtin ("atomic_store_16", BUILT_IN_ATOMIC_STORE_16,
-		  "__atomic_store_16", NULL,
-		  build_varargs_function_type_list (void_type_node, NULL_TREE),
-		  0);
+		  "__atomic_store_16", NULL, atomic_store_type, 0);
+
+  define_builtin ("atomic_load_1", BUILT_IN_ATOMIC_LOAD_1, "__atomic_load_1",
+		  NULL, atomic_load_type (integer_type_node), 0);
+  define_builtin ("atomic_load_2", BUILT_IN_ATOMIC_LOAD_2, "__atomic_load_2",
+		  NULL, atomic_load_type (integer_type_node), 0);
+  define_builtin ("atomic_load_4", BUILT_IN_ATOMIC_LOAD_4, "__atomic_load_4",
+		  NULL, atomic_load_type (integer_type_node), 0);
+  define_builtin ("atomic_load_8", BUILT_IN_ATOMIC_LOAD_8, "__atomic_load_8",
+		  NULL, atomic_load_type (integer_type_node), 0);
 }
 
 void
diff --git a/gcc/rust/backend/rust-compile-intrinsic.cc b/gcc/rust/backend/rust-compile-intrinsic.cc
index 142a217..5522211 100644
--- a/gcc/rust/backend/rust-compile-intrinsic.cc
+++ b/gcc/rust/backend/rust-compile-intrinsic.cc
@@ -50,6 +50,22 @@ is_basic_integer_type (TyTy::BaseType *type)
     }
 }
 
+static bool
+check_for_basic_integer_type (const std::string &intrinsic_str, Location locus,
+			      TyTy::BaseType *type)
+{
+  auto is_basic_integer = is_basic_integer_type (type);
+  if (!is_basic_integer)
+    {
+      rust_error_at (
+	locus,
+	"%s intrinsics can only be used with basic integer types (got %qs)",
+	intrinsic_str.c_str (), type->get_name ().c_str ());
+    }
+
+  return is_basic_integer;
+}
+
 static tree
 offset_handler (Context *ctx, TyTy::FnType *fntype);
 static tree
@@ -104,6 +120,8 @@ prefetch_write_data (Context *ctx, TyTy::FnType *fntype)
 
 static tree
 atomic_store_handler_inner (Context *ctx, TyTy::FnType *fntype, int ordering);
+static tree
+atomic_load_handler_inner (Context *ctx, TyTy::FnType *fntype, int ordering);
 
 static inline std::function<tree (Context *, TyTy::FnType *)>
 atomic_store_handler (int ordering)
@@ -113,6 +131,14 @@ atomic_store_handler (int ordering)
   };
 }
 
+static inline std::function<tree (Context *, TyTy::FnType *)>
+atomic_load_handler (int ordering)
+{
+  return [ordering] (Context *ctx, TyTy::FnType *fntype) {
+    return atomic_load_handler_inner (ctx, fntype, ordering);
+  };
+}
+
 static inline tree
 unchecked_op_inner (Context *ctx, TyTy::FnType *fntype, tree_code op);
 
@@ -124,7 +150,7 @@ unchecked_op_handler (tree_code op)
   };
 }
 
-static inline tree
+inline tree
 sorry_handler (Context *ctx, TyTy::FnType *fntype)
 {
   rust_sorry_at (fntype->get_locus (), "intrinsic %qs is not yet implemented",
@@ -147,11 +173,14 @@ static const std::map<std::string,
     {"copy_nonoverlapping", copy_nonoverlapping_handler},
     {"prefetch_read_data", prefetch_read_data},
     {"prefetch_write_data", prefetch_write_data},
-    {"atomic_load", sorry_handler},
     {"atomic_store_seqcst", atomic_store_handler (__ATOMIC_SEQ_CST)},
     {"atomic_store_release", atomic_store_handler (__ATOMIC_RELEASE)},
     {"atomic_store_relaxed", atomic_store_handler (__ATOMIC_RELAXED)},
     {"atomic_store_unordered", atomic_store_handler (__ATOMIC_RELAXED)},
+    {"atomic_load_seqcst", atomic_load_handler (__ATOMIC_SEQ_CST)},
+    {"atomic_load_acquire", atomic_load_handler (__ATOMIC_ACQUIRE)},
+    {"atomic_load_relaxed", atomic_load_handler (__ATOMIC_RELAXED)},
+    {"atomic_load_unordered", atomic_load_handler (__ATOMIC_RELAXED)},
     {"unchecked_add", unchecked_op_handler (PLUS_EXPR)},
     {"unchecked_sub", unchecked_op_handler (MINUS_EXPR)},
     {"unchecked_mul", unchecked_op_handler (MULT_EXPR)},
@@ -652,7 +681,8 @@ prefetch_data_handler (Context *ctx, TyTy::FnType *fntype, Prefetch kind)
 }
 
 static std::string
-build_atomic_builtin_name (Location locus, tree operand_type)
+build_atomic_builtin_name (const std::string &prefix, Location locus,
+			   TyTy::BaseType *operand_type)
 {
   static const std::map<std::string, std::string> allowed_types = {
     {"i8", "1"},    {"i16", "2"},   {"i32", "4"},   {"i64", "8"},
@@ -663,9 +693,9 @@ build_atomic_builtin_name (Location locus, tree operand_type)
   // TODO: Can we maybe get the generic version (atomic_store_n) to work... This
   // would be so much better
 
-  std::string result = "atomic_store_";
+  std::string result = prefix;
 
-  auto type_name = std::string (TYPE_NAME_STRING (operand_type));
+  auto type_name = operand_type->get_name ();
   if (type_name == "usize" || type_name == "isize")
     {
       rust_sorry_at (
@@ -673,21 +703,10 @@ build_atomic_builtin_name (Location locus, tree operand_type)
       return "";
     }
 
-  // FIXME: Can we have a better looking name here?
-  // Instead of `<crate>::<module>::<type>`?
-  // Maybe instead of giving the tree node, pass the resolved Tyty before it
-  // gets compiled?
-  //
-  // Or should we perform this check somwhere else in the compiler?
   auto type_size_str = allowed_types.find (type_name);
-  if (type_size_str == allowed_types.end ())
-    {
-      rust_error_at (locus,
-		     "atomic intrinsics are only available for basic integer "
-		     "types: got type %qs",
-		     type_name.c_str ());
-      return "";
-    }
+
+  if (!check_for_basic_integer_type ("atomic", locus, operand_type))
+    return "";
 
   result += type_size_str->second;
 
@@ -726,8 +745,12 @@ atomic_store_handler_inner (Context *ctx, TyTy::FnType *fntype, int ordering)
   auto value = ctx->get_backend ()->var_expression (param_vars[1], Location ());
   auto memorder = make_unsigned_long_tree (ctx, ordering);
 
+  auto monomorphized_type
+    = fntype->get_substs ()[0].get_param_ty ()->resolve ();
+
   auto builtin_name
-    = build_atomic_builtin_name (fntype->get_locus (), TREE_TYPE (types[0]));
+    = build_atomic_builtin_name ("atomic_store_", fntype->get_locus (),
+				 monomorphized_type);
   if (builtin_name.empty ())
     return error_mark_node;
 
@@ -743,7 +766,6 @@ atomic_store_handler_inner (Context *ctx, TyTy::FnType *fntype, int ordering)
     = ctx->get_backend ()->call_expression (atomic_store,
 					    {dst, value, memorder}, nullptr,
 					    Location ());
-
   TREE_READONLY (store_call) = 0;
   TREE_SIDE_EFFECTS (store_call) = 1;
 
@@ -753,6 +775,68 @@ atomic_store_handler_inner (Context *ctx, TyTy::FnType *fntype, int ordering)
   return fndecl;
 }
 
+static tree
+atomic_load_handler_inner (Context *ctx, TyTy::FnType *fntype, int ordering)
+{
+  rust_assert (fntype->get_params ().size () == 1);
+  rust_assert (fntype->get_num_substitutions () == 1);
+
+  tree lookup = NULL_TREE;
+  if (check_for_cached_intrinsic (ctx, fntype, &lookup))
+    return lookup;
+
+  auto fndecl = compile_intrinsic_function (ctx, fntype);
+
+  // Most intrinsic functions are pure but not the atomic ones
+  // FIXME: Is atomic_load_* pure? Feels like it shouldn't so
+  TREE_READONLY (fndecl) = 0;
+  TREE_SIDE_EFFECTS (fndecl) = 1;
+
+  // setup the params
+  std::vector<Bvariable *> param_vars;
+  std::vector<tree> types;
+  compile_fn_params (ctx, fntype, fndecl, &param_vars, &types);
+
+  auto ok = ctx->get_backend ()->function_set_parameters (fndecl, param_vars);
+  rust_assert (ok);
+
+  enter_intrinsic_block (ctx, fndecl);
+
+  auto src = ctx->get_backend ()->var_expression (param_vars[0], Location ());
+  auto memorder = make_unsigned_long_tree (ctx, ordering);
+
+  auto monomorphized_type
+    = fntype->get_substs ()[0].get_param_ty ()->resolve ();
+
+  auto builtin_name
+    = build_atomic_builtin_name ("atomic_load_", fntype->get_locus (),
+				 monomorphized_type);
+  if (builtin_name.empty ())
+    return error_mark_node;
+
+  tree atomic_load_raw = nullptr;
+  BuiltinsContext::get ().lookup_simple_builtin (builtin_name,
+						 &atomic_load_raw);
+  rust_assert (atomic_load_raw);
+
+  auto atomic_load
+    = build_fold_addr_expr_loc (Location ().gcc_location (), atomic_load_raw);
+
+  auto load_call
+    = ctx->get_backend ()->call_expression (atomic_load, {src, memorder},
+					    nullptr, Location ());
+  auto return_statement
+    = ctx->get_backend ()->return_statement (fndecl, {load_call}, Location ());
+
+  TREE_READONLY (load_call) = 0;
+  TREE_SIDE_EFFECTS (load_call) = 1;
+
+  ctx->add_statement (return_statement);
+  finalize_intrinsic_block (ctx, fndecl);
+
+  return fndecl;
+}
+
 static inline tree
 unchecked_op_inner (Context *ctx, TyTy::FnType *fntype, tree_code op)
 {
@@ -781,11 +865,9 @@ unchecked_op_inner (Context *ctx, TyTy::FnType *fntype, tree_code op)
 
   auto *monomorphized_type
     = fntype->get_substs ().at (0).get_param_ty ()->resolve ();
-  if (!is_basic_integer_type (monomorphized_type))
-    rust_error_at (fntype->get_locus (),
-		   "unchecked operation intrinsics can only be used with "
-		   "basic integer types (got %qs)",
-		   monomorphized_type->get_name ().c_str ());
+
+  check_for_basic_integer_type ("unchecked operation", fntype->get_locus (),
+				monomorphized_type);
 
   auto expr = build2 (op, TREE_TYPE (x), x, y);
   auto return_statement
diff --git a/gcc/testsuite/rust/compile/torture/intrinsics-3.rs b/gcc/testsuite/rust/compile/torture/intrinsics-3.rs
index 1acb353..5c131bd 100644
--- a/gcc/testsuite/rust/compile/torture/intrinsics-3.rs
+++ b/gcc/testsuite/rust/compile/torture/intrinsics-3.rs
@@ -1,9 +1,7 @@
 extern "rust-intrinsic" {
     fn not_an_intrinsic();
-    fn atomic_load(); // { dg-message "sorry, unimplemented: intrinsic .atomic_load. is not yet implemented" }
 }
 
 fn main() {
     unsafe { not_an_intrinsic() }; // { dg-error "unknown builtin intrinsic: not_an_intrinsic" }
-    unsafe { atomic_load() };
 }
diff --git a/gcc/testsuite/rust/compile/torture/intrinsics-5.rs b/gcc/testsuite/rust/compile/torture/intrinsics-5.rs
index e008772..7fd84dc 100644
--- a/gcc/testsuite/rust/compile/torture/intrinsics-5.rs
+++ b/gcc/testsuite/rust/compile/torture/intrinsics-5.rs
@@ -2,8 +2,8 @@ trait Copy {}
 
 extern "rust-intrinsic" {
     pub fn atomic_store_seqcst<T: Copy>(dst: *mut T, value: T);
-    // { dg-error "atomic intrinsics are only available for basic integer types: got type .intrinsics_5::VeryLargeType." "" { target *-*-* } .-1 }
-    // { dg-error "atomic intrinsics are only available for basic integer types: got type .bool." "" { target *-*-* } .-2 }
+    // { dg-error "atomic intrinsics can only be used with basic integer types .got .VeryLargeType.." "" { target *-*-* } .-1 }
+    // { dg-error "atomic intrinsics can only be used with basic integer types .got .bool.." "" { target *-*-* } .-2 }
 }
 
 struct VeryLargeType {
diff --git a/gcc/testsuite/rust/execute/torture/atomic_load.rs b/gcc/testsuite/rust/execute/torture/atomic_load.rs
new file mode 100644
index 0000000..28ed8ae
--- /dev/null
+++ b/gcc/testsuite/rust/execute/torture/atomic_load.rs
@@ -0,0 +1,31 @@
+trait Copy {}
+
+extern "rust-intrinsic" {
+    pub fn atomic_load_seqcst<T: Copy>(src: *const T) -> T;
+    pub fn atomic_load_acquire<T: Copy>(src: *const T) -> T;
+    pub fn atomic_load_relaxed<T: Copy>(src: *const T) -> T;
+    pub fn atomic_load_unordered<T: Copy>(src: *const T) -> T;
+}
+
+fn main() -> i32 {
+    let one;
+    let two;
+    let three;
+    let four;
+
+    unsafe {
+        let mut src = 1;
+        one = atomic_load_seqcst(&src);
+
+        src = 2;
+        two = atomic_load_acquire(&src);
+
+        src = 3;
+        three = atomic_load_relaxed(&src);
+
+        src = 4;
+        four = atomic_load_unordered(&src);
+    }
+
+    (four + three + two + one) - 10
+}