From: Arthur Cohen <[email protected]>

The __builtin_prefetch() builtin currently does not support an extra argument 
to indicate
which cache to use, as opposed to LLVM's llvm.prefetch intrinsic. Prefetches 
are allowed
to be no-ops, since they are just a performance hint - so for now, just no-op 
when attempting
to prefetch an instruction.

gcc/rust/ChangeLog:

        * backend/rust-compile-intrinsic.cc: Rework prefetch handlers, add them 
for
        prefetch_{read, write}_instruction
        * backend/rust-intrinsic-handlers.cc (prefetch): New function, to 
replace...
        (prefetch_read_data): ...this one...
        (prefetch_write_data): ...and this one...
        (prefetch_data): ...and this one!
        * backend/rust-intrinsic-handlers.h (enum class Prefetch): Rename to...
        (enum class Mode): ...this.
        (enum class Cache): New enum.
        (prefetch_data): Remove.
        (prefetch_read_data): Likewise.
        (prefetch_write_data): Likewise.
        (prefetch): New handler and internal implementation.
        * util/rust-intrinsic-values.h: Add PREFETCH_{READ, WRITE}_INSTRUCTION.
        * typecheck/rust-hir-type-check-intrinsic.cc: Add entries for
        prefetch_{read, write}_instruction

gcc/testsuite/ChangeLog:

        * rust/compile/prefetch_all.rs: New test.

Tested on x86_64-pc-linux-gnu.

---
 gcc/rust/backend/rust-compile-intrinsic.cc    |   8 +-
 gcc/rust/backend/rust-intrinsic-handlers.cc   | 118 ++++++++++--------
 gcc/rust/backend/rust-intrinsic-handlers.h    |  16 ++-
 .../rust-hir-type-check-intrinsic.cc          |  10 +-
 gcc/rust/util/rust-intrinsic-values.h         |   4 +
 gcc/testsuite/rust/compile/prefetch_all.rs    |  25 ++++
 6 files changed, 118 insertions(+), 63 deletions(-)
 create mode 100644 gcc/testsuite/rust/compile/prefetch_all.rs

diff --git a/gcc/rust/backend/rust-compile-intrinsic.cc 
b/gcc/rust/backend/rust-compile-intrinsic.cc
index 994dda03cc1..a1ee04de768 100644
--- a/gcc/rust/backend/rust-compile-intrinsic.cc
+++ b/gcc/rust/backend/rust-compile-intrinsic.cc
@@ -46,8 +46,12 @@ static const std::map<std::string, Intrinsic::CompileFn> 
intrinsics = {
   {IValue::MUL_WITH_OVERFLOW, handlers::op_with_overflow (MULT_EXPR)},
   {IValue::COPY, handlers::copy (true)},
   {IValue::COPY_NONOVERLAPPING, handlers::copy (false)},
-  {IValue::PREFETCH_READ_DATA, handlers::prefetch_read_data},
-  {IValue::PREFETCH_WRITE_DATA, handlers::prefetch_write_data},
+  {IValue::PREFETCH_READ_DATA, handlers::prefetch (Mode::Read, Cache::Data)},
+  {IValue::PREFETCH_WRITE_DATA, handlers::prefetch (Mode::Write, Cache::Data)},
+  {IValue::PREFETCH_READ_INSTRUCTION,
+   handlers::prefetch (Mode::Read, Cache::Instruction)},
+  {IValue::PREFETCH_WRITE_INSTRUCTION,
+   handlers::prefetch (Mode::Write, Cache::Instruction)},
   {IValue::ATOMIC_STORE, handlers::atomic_store (__ATOMIC_SEQ_CST)},
   {IValue::ATOMIC_STORE_SEQCST, handlers::atomic_store (__ATOMIC_SEQ_CST)},
   {IValue::ATOMIC_STORE_REL, handlers::atomic_store (__ATOMIC_RELEASE)},
diff --git a/gcc/rust/backend/rust-intrinsic-handlers.cc 
b/gcc/rust/backend/rust-intrinsic-handlers.cc
index dcaf046a479..4ace3fcf4a7 100644
--- a/gcc/rust/backend/rust-intrinsic-handlers.cc
+++ b/gcc/rust/backend/rust-intrinsic-handlers.cc
@@ -1193,6 +1193,62 @@ type_assert (IntrinsicCtx &ctx, TyAssert assert)
     }
 }
 
+void
+prefetch (IntrinsicCtx &ctx, Mode mode, Cache cache)
+{
+  // The core library technically allows you to pass any i32 value as a
+  // locality, but LLVM will then complain if the value cannot be constant
+  // evaluated. For now, we ignore the locality argument and instead always
+  // pass `3` (the most restrictive value). This allows us to still have
+  // prefetch behavior, just not as granular as expected. In future Rust
+  // versions, we hope that prefetch intrinsics will be split up according to
+  // locality, similarly to atomic intrinsics.
+  // The solution is to try and perform constant folding for the locality
+  // argument, or instead of creating a new function definition, modify the 
call
+  // site directly This has the bad side-effect of creating warnings about
+  // `unused name - locality`, which we hack away here:
+  // TODO: Take care of handling locality properly
+  Backend::var_expression (ctx.param_vars[1], UNDEF_LOCATION);
+
+  // If we are doing prefetches on the instruction cache, then emit a no-op. 
GCC
+  // currently has no support for prefetching the instruction cache, and
+  // prefetches are allowed to be completely ignored anyways. Revisit once
+  // __builtin_prefetch() accepts another argument for the `cache` to use, in
+  // the same way that llvm.prefetch does. In the same vein as above, mark the
+  // given addr as used so there's no complaints.
+  if (cache == Cache::Instruction)
+    {
+      Backend::var_expression (ctx.param_vars[0], UNDEF_LOCATION);
+
+      return;
+    }
+
+  // prefetching isn't pure and shouldn't be discarded after GIMPLE
+  TREE_READONLY (ctx.fn) = 0;
+  TREE_SIDE_EFFECTS (ctx.fn) = 1;
+
+  auto addr = Backend::var_expression (ctx.param_vars[0], UNDEF_LOCATION);
+
+  auto rw_flag = make_unsigned_long_tree (mode == Mode::Write ? 1 : 0);
+
+  auto prefetch_raw
+    = BuiltinsContext::get ().lookup_simple_builtin ("__builtin_prefetch");
+  rust_assert (prefetch_raw);
+
+  auto prefetch = build_fold_addr_expr_loc (UNKNOWN_LOCATION, *prefetch_raw);
+
+  auto prefetch_call = Backend::call_expression (prefetch,
+                                                {addr, rw_flag,
+                                                 // locality arg
+                                                 make_unsigned_long_tree (3)},
+                                                nullptr, UNDEF_LOCATION);
+
+  TREE_READONLY (prefetch_call) = 0;
+  TREE_SIDE_EFFECTS (prefetch_call) = 1;
+
+  ctx.add_statement (prefetch_call);
+}
+
 } // namespace inner
 
 Intrinsic::CompileFn
@@ -1346,6 +1402,14 @@ try_handler (bool is_new_api)
   };
 }
 
+Intrinsic::CompileFn
+prefetch (Mode mode, Cache cache)
+{
+  return [mode, cache] (IntrinsicCtx &ctx) {
+    return inner::prefetch (ctx, mode, cache);
+  };
+}
+
 void
 sorry (IntrinsicCtx &ctx)
 {
@@ -1520,60 +1584,6 @@ uninit (IntrinsicCtx &ctx)
   ctx.add_statement (return_statement);
 }
 
-void
-prefetch_read_data (IntrinsicCtx &ctx)
-{
-  return prefetch_data (ctx, Prefetch::Read);
-}
-void
-prefetch_write_data (IntrinsicCtx &ctx)
-{
-  return prefetch_data (ctx, Prefetch::Write);
-}
-
-void
-prefetch_data (IntrinsicCtx &ctx, Prefetch kind)
-{
-  // prefetching isn't pure and shouldn't be discarded after GIMPLE
-  TREE_READONLY (ctx.fn) = 0;
-  TREE_SIDE_EFFECTS (ctx.fn) = 1;
-
-  auto addr = Backend::var_expression (ctx.param_vars[0], UNDEF_LOCATION);
-
-  // The core library technically allows you to pass any i32 value as a
-  // locality, but LLVM will then complain if the value cannot be constant
-  // evaluated. For now, we ignore the locality argument and instead always
-  // pass `3` (the most restrictive value). This allows us to still have
-  // prefetch behavior, just not as granular as expected. In future Rust
-  // versions, we hope that prefetch intrinsics will be split up according to
-  // locality, similarly to atomic intrinsics.
-  // The solution is to try and perform constant folding for the locality
-  // argument, or instead of creating a new function definition, modify the 
call
-  // site directly This has the bad side-effect of creating warnings about
-  // `unused name - locality`, which we hack away here:
-  // TODO: Take care of handling locality properly
-  Backend::var_expression (ctx.param_vars[1], UNDEF_LOCATION);
-
-  auto rw_flag = make_unsigned_long_tree (kind == Prefetch::Write ? 1 : 0);
-
-  auto prefetch_raw
-    = BuiltinsContext::get ().lookup_simple_builtin ("__builtin_prefetch");
-  rust_assert (prefetch_raw);
-
-  auto prefetch = build_fold_addr_expr_loc (UNKNOWN_LOCATION, *prefetch_raw);
-
-  auto prefetch_call = Backend::call_expression (prefetch,
-                                                {addr, rw_flag,
-                                                 // locality arg
-                                                 make_unsigned_long_tree (3)},
-                                                nullptr, UNDEF_LOCATION);
-
-  TREE_READONLY (prefetch_call) = 0;
-  TREE_SIDE_EFFECTS (prefetch_call) = 1;
-
-  ctx.add_statement (prefetch_call);
-}
-
 void
 rotate (IntrinsicCtx &ctx, tree_code op)
 {
diff --git a/gcc/rust/backend/rust-intrinsic-handlers.h 
b/gcc/rust/backend/rust-intrinsic-handlers.h
index 8d5cb9f73ec..cdf753be952 100644
--- a/gcc/rust/backend/rust-intrinsic-handlers.h
+++ b/gcc/rust/backend/rust-intrinsic-handlers.h
@@ -115,12 +115,18 @@ private:
   CompileFn closure;
 };
 
-enum class Prefetch
+enum class Mode
 {
   Read,
   Write
 };
 
+enum class Cache
+{
+  Data,
+  Instruction,
+};
+
 enum class TyAssert
 {
   ZeroValid,
@@ -156,6 +162,8 @@ void op_with_overflow (IntrinsicCtx &ctx, tree_code op);
 
 inline void unchecked_op (IntrinsicCtx &ctx, tree_code op);
 
+void prefetch (IntrinsicCtx &ctx, Mode mode, Cache cache);
+
 } // namespace inner
 
 void rotate_left (IntrinsicCtx &ctx);
@@ -171,7 +179,6 @@ void transmute (IntrinsicCtx &ctx);
 // FIXME: This is supposed to be just a ctx without the op right?
 // Or are these inner functions and thus shouldn't live here?
 void rotate (IntrinsicCtx &ctx, tree_code op);
-void prefetch_data (IntrinsicCtx &ctx, Prefetch kind);
 
 void uninit (IntrinsicCtx &ctx);
 void move_val_init (IntrinsicCtx &ctx);
@@ -184,9 +191,6 @@ void ctlz_nonzero_handler (IntrinsicCtx &ctx);
 void cttz_handler (IntrinsicCtx &ctx);
 void cttz_nonzero_handler (IntrinsicCtx &ctx);
 
-void prefetch_read_data (IntrinsicCtx &ctx);
-void prefetch_write_data (IntrinsicCtx &ctx);
-
 void sorry (IntrinsicCtx &ctx);
 
 void float_to_int_unchecked (IntrinsicCtx &ctx);
@@ -222,6 +226,8 @@ Intrinsic::CompileFn copy (bool overlaps);
 Intrinsic::CompileFn expect (bool likely);
 Intrinsic::CompileFn try_handler (bool is_new_api);
 
+Intrinsic::CompileFn prefetch (Mode mode, Cache cache);
+
 /**
  * For fadd_fast, fsub_fast, fmul_fast, fdiv_fast and frem_fast
  */
diff --git a/gcc/rust/typecheck/rust-hir-type-check-intrinsic.cc 
b/gcc/rust/typecheck/rust-hir-type-check-intrinsic.cc
index c423cbb4377..56a4888dc00 100644
--- a/gcc/rust/typecheck/rust-hir-type-check-intrinsic.cc
+++ b/gcc/rust/typecheck/rust-hir-type-check-intrinsic.cc
@@ -157,12 +157,18 @@ const std::unordered_map<std::string, IntrinsicRules>
      {1,
       {IRT::ConstPtrFirstGeneric, IRT::MutPtrFirstGeneric, IRT::Usize},
       IRT::Unit}},
-    // pub fn prefetch_read_data<T>(data: *const T, locality: i32);
+
+    // pub fn prefetch_{read,write}_{data,instruction}<T>(data: *const T,
+    // locality: i32);
     {IValue::PREFETCH_READ_DATA,
      {1, {IRT::ConstPtrFirstGeneric, IRT::I32}, IRT::Unit}},
-    // pub fn prefetch_write_data<T>(data: *const T, locality: i32);
     {IValue::PREFETCH_WRITE_DATA,
      {1, {IRT::ConstPtrFirstGeneric, IRT::I32}, IRT::Unit}},
+    {IValue::PREFETCH_READ_INSTRUCTION,
+     {1, {IRT::ConstPtrFirstGeneric, IRT::I32}, IRT::Unit}},
+    {IValue::PREFETCH_WRITE_INSTRUCTION,
+     {1, {IRT::ConstPtrFirstGeneric, IRT::I32}, IRT::Unit}},
+
     // pub unsafe fn atomic_store_seqcst<T: Copy>(_dst: *mut T, _val: T);
     {IValue::ATOMIC_STORE_SEQCST,
      {1, {IRT::MutPtrFirstGeneric, IRT::FirstGeneric}, IRT::Unit}},
diff --git a/gcc/rust/util/rust-intrinsic-values.h 
b/gcc/rust/util/rust-intrinsic-values.h
index 2560739b861..d2dad8d3a78 100644
--- a/gcc/rust/util/rust-intrinsic-values.h
+++ b/gcc/rust/util/rust-intrinsic-values.h
@@ -96,6 +96,10 @@ public:
 
   static constexpr auto &PREFETCH_READ_DATA = "prefetch_read_data";
   static constexpr auto &PREFETCH_WRITE_DATA = "prefetch_write_data";
+  static constexpr auto &PREFETCH_READ_INSTRUCTION
+    = "prefetch_read_instruction";
+  static constexpr auto &PREFETCH_WRITE_INSTRUCTION
+    = "prefetch_write_instruction";
 
   static constexpr auto &ATOMIC_STORE = "atomic_store";
   // atomic_store is renamed to atomic_store_seqcst from 1.64.0, the latter is
diff --git a/gcc/testsuite/rust/compile/prefetch_all.rs 
b/gcc/testsuite/rust/compile/prefetch_all.rs
new file mode 100644
index 00000000000..0cea6671800
--- /dev/null
+++ b/gcc/testsuite/rust/compile/prefetch_all.rs
@@ -0,0 +1,25 @@
+#![feature(no_core)]
+#![feature(intrinsics)]
+#![feature(lang_items)]
+#![no_core]
+
+#[lang = "sized"]
+trait Sized {}
+
+extern "rust-intrinsic" {
+    fn prefetch_read_data<T>(data: *const T, locality: i32);
+    fn prefetch_write_data<T>(data: *const T, locality: i32);
+    fn prefetch_read_instruction<T>(instruction: *const T, locality: i32);
+    fn prefetch_write_instruction<T>(instruction: *const T, locality: i32);
+}
+
+fn main() {
+    let a = 15i32;
+
+    unsafe {
+        prefetch_read_data(&a as *const i32, 3);
+        prefetch_write_data(&a as *const i32, 3);
+        prefetch_read_instruction(&a as *const i32, 3);
+        prefetch_write_instruction(&a as *const i32, 3);
+    }
+}
-- 
2.55.0

Reply via email to