[PATCH] RISC-V: Implement __clear_cache via __builtin__clear_cache

[PATCH] RISC-V: Implement __clear_cache via __builtin__clear_cache

[Palmer Dabbelt]

We have had an implementation of __builtin__clear_cache since the
beginning, but didn't have the cooresponding __clear_cache library
routine implemented.  This directly conflicts the GCC manual in a
handful of places, which indicates that __clear_cache should work and
that __builtin_clear_cache should function the same way as
__clear_cache by ethier calling it or inlining the functionality.

This patch simply implements __clear_cache via __builtin__clear_cache.
This should be safe as we always have clear_cache insn so therefor
__builtin__clear_cache will never fall back to calling __clear_cache.
I'm not actually sure that silently implementing clear_cache as a NOP
when there is no ISA defined mechanism for icache synchronization is the
right way to go, but that's really a different discussion.

This was reported as Bug 94136, which is a year old but was categorized
as a documentation bug.  I believe that categorization was incorrect:
having an empty __clear_cache library routine is simply incorrect
behavior, the fact that __builtin__clear_cache happens to be implemented
as a libc call on Linux is just a red herring suggesting the
documentation fix to point out the name difference.  I view this new
behavior as conforming to the existing documentation: we're just
inlining the __clear_cache implementation, even if that implementation
happens to be a call to a very similar looking libc routine.

gcc/ChangeLog
	PR target/94136
	* config/riscv/riscv.h (CLEAR_INSN_CACHE): New macro.

---

Something has gone off the rails with my laptop and it's failing to
build GCC, so I haven't actually tested this.  I'm not sure it's sane to
call a GCC builtin from within libgcc, but I figured it would be best to
just send out the patch to ask.
---
 gcc/config/riscv/riscv.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/gcc/config/riscv/riscv.h b/gcc/config/riscv/riscv.h
index f3e85723c85..39a688ea1e9 100644
--- a/gcc/config/riscv/riscv.h
+++ b/gcc/config/riscv/riscv.h
@@ -993,4 +993,9 @@ extern void riscv_remove_unneeded_save_restore_calls (void);
 
 #define HARD_REGNO_RENAME_OK(FROM, TO) riscv_hard_regno_rename_ok (FROM, TO)
 
+/* We always have a "clear_cache" insn, which means __builtin__clear_cache will
+   never emit a call to __clear_cache.  */
+#undef CLEAR_INSN_CACHE
+#define CLEAR_INSN_CACHE(BEG, END) __builtin__clear_cache((BEG), (END));
+
 #endif /* ! GCC_RISCV_H */
-- 
2.31.1.527.g47e6f16901-goog

Re: [PATCH] RISC-V: Implement __clear_cache via __builtin__clear_cache

[Jim Wilson]

On Thu, Apr 29, 2021 at 10:02 PM Palmer Dabbelt <palmerdabbelt@google.com>
wrote:

> This was reported as Bug 94136, which is a year old but was categorized
> as a documentation bug.  I believe that categorization was incorrect:
> having an empty __clear_cache library routine is simply incorrect


It affects almost all targets, and fixing RISC-V does nothing useful to
change that.  x86_64-linux for instance also has an empty __clear_cache
function in libgcc.a.  We can either fix almost all targets, or we can fix
the docs to match reality.  Fixing the docs is easier.  This is why it is
classified as a doc bug.

Fixing riscv is nice.  You can remove the riscv target setting.  But you
can't close it because it is still broken for a lot of other targets.
Unless maybe this trick of defining CLEAR_INSN_CACHE works for other
targets too, in which case maybe we could add a default definition
someplace for targets that don't have their own definition that does what
your RISC-V patch does.

+/* We always have a "clear_cache" insn, which means __builtin__clear_cache
> will
> +   never emit a call to __clear_cache.  */
> +#undef CLEAR_INSN_CACHE
> +#define CLEAR_INSN_CACHE(BEG, END) __builtin__clear_cache((BEG), (END));
>

It is __builtin___clear_cache with 3 underscores before the clear.  With
this change it works for me.

Jim