introduce overridable clear_cache emitter

introduce overridable clear_cache emitter

[Alexandre Oliva]

This patch introduces maybe_emit_call_builtin___clear_cache for the
builtin expander machinery and the trampoline initializers to use to
clear the instruction cache, removing a source of inconsistencies and
subtle errors in low-level machinery.

I've adjusted all trampoline_init implementations that used to issue
explicit calls to __clear_cache or similar to use this new primitive.


Specifically on vxworks targets, we needed to drop the __clear_cache
symbol in libgcc, for reasons related with linking that I didn't need
to understand, and we wanted to call cacheTextUpdate directly, despite
the different calling conventions: the second argument is a length
rather than the end address.

So I introduced a target hook to enable target OS-level overriding of
builtin __clear_cache call emission, retaining nearly (*) the same
logic to govern the decision on whether to emit a call (or nothing, or
a machine-dependent insn) but enabling a call to a target
system-defined function with different calling conventions to be
issued, without having to modify .md files of the various
architectures supported by the target system to introduce or modify
clear_cache insns.

(*) I write "nearly" mainly because, when not optimizing, we'd issue a
call regardless, but since the call may now be overridden, I added it
to the set of builtins that are not directly turned into calls when
not optimizing, following the normal expansion path instead.  It
wouldn't be hard to skip the emission of cache-clearing insns when not
optimizing, but it didn't seem very important, especially for the new
uses from trampoline init.

    Another difference that might be relevant is that now we expand
the begin and end arguments unconditionally.  This might make a
difference if they have side effects.  That's prettty much impossible
at expand time, but I thought I'd mention it.


I have NOT modified targets that did not issue cache-clearing calls in
trampoline init to use the new clear_cache-calling infrastructure even
if it would expand to nothing.  I have considered doing so, to have
__builtin___clear_cache and trampoline init call cacheTextUpdate on
all vxworks targets, but decided not to, since on targets that don't
do any cache clearing, cacheTextUpdate ought to be a no-op, even
though rs6000 seems to use icbi and dcbf instructions in the function
called to initialize a trampoline, but AFAICT not in the __clear_cache
builtin.  Hopefully target maintainers will have a look and take
advantage of this new piece of infrastructure to remove such
(apparent?) inconsistencies.  Not rs6000 and other that call asm-coded
trampoline setup instructions, for sure, but they might wish to
introduce a CLEAR_INSN_CACHE macro or a clear_cache expander if they
don't have one.


This was regstrapped on x86_64-linux-gnu, and a trivial backport was
tested on multiple vxworks targets.  Ok to install?


for  gcc/ChangeLog

	* builtins.c (default_emit_call_builtin___clear_cache): New.
	(maybe_emit_call_builtin___clear_cache): New.
	(expand_builtin___clear_cache): Split into the above.
	(expand_builtin): Do not issue clear_cache call any more.
	* builtins.h (maybe_emit_call_builtin___clear_cache): Declare.
	* config/aarch64/aarch64.c (aarch64_trampoline_init): Use
	maybe_emit_call_builtin___clear_cache.
	* config/arc/arc.c (arc_trampoline_init): Likewise.
	* config/arm/arm.c (arm_trampoline_init): Likewise.
	* config/c6x/c6x.c (c6x_initialize_trampoline): Likewise.
	* config/csky/csky.c (csky_trampoline_init): Likewise.
	* config/m68k/linux.h (FInALIZE_TRAMPOLINE): Likewise.
	* config/tilegx/tilegx.c (tilegx_trampoline_init): Likewise.
	* config/tilepro/tilepro.c (tilepro_trampoline_init): Ditto.
	* config/vxworks.c: Include rtl.h, memmodel.h, and optabs.h.
	(vxworks_emit_call_builtin___clear_cache): New.
	* config/vxworks.h (CLEAR_INSN_CACHE): Drop.
	(TARGET_EMIT_CALL_BUILTIN___CLEAR_CACHE): Define.
	* target.def (trampoline_init): In the documentation, refer to
	maybe_emit_call_builtin___clear_cache.
	(emit_call_builtin___clear_cache): New.
	* doc/tm.texi.in: Add new hook point.
	(CLEAR_CACHE_INSN): Remove duplicate 'both'.
	* doc/tm.texi: Rebuilt.
	* targhooks.h (default_meit_call_builtin___clear_cache):
	Declare.
	* tree.h (BUILTIN_ASM_NAME_PTR): New.

for  libgcc/ChangeLog

	* config/t-vxworks (LIB2ADD): Drop.
	* config/t-vxworks7 (LIB2ADD): Likewise.
	* config/vxcache.c: Remove.
---
 gcc/builtins.c               |   89 +++++++++++++++++++++++++++---------------
 gcc/builtins.h               |    1 
 gcc/config/aarch64/aarch64.c |    8 ++--
 gcc/config/arc/arc.c         |    8 ++--
 gcc/config/arm/arm.c         |    7 ++-
 gcc/config/c6x/c6x.c         |    8 ++--
 gcc/config/csky/csky.c       |    7 ++-
 gcc/config/m68k/linux.h      |    8 ++--
 gcc/config/tilegx/tilegx.c   |    4 --
 gcc/config/tilepro/tilepro.c |    4 --
 gcc/config/vxworks.c         |   24 +++++++++++
 gcc/config/vxworks.h         |   11 +++--
 gcc/doc/tm.texi              |   17 +++++++-
 gcc/doc/tm.texi.in           |    4 +-
 gcc/target.def               |   18 ++++++++
 gcc/targhooks.h              |    1 
 gcc/tree.h                   |    7 +++
 libgcc/config/t-vxworks      |    1 
 libgcc/config/t-vxworks7     |    1 
 libgcc/config/vxcache.c      |   35 -----------------
 20 files changed, 160 insertions(+), 103 deletions(-)
 delete mode 100644 libgcc/config/vxcache.c

diff --git a/gcc/builtins.c b/gcc/builtins.c
index da25343beb1b9..052a7a21bd715 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -7053,26 +7053,63 @@ expand_builtin_copysign (tree exp, rtx target, rtx subtarget)
   return expand_copysign (op0, op1, target);
 }
 
-/* Expand a call to __builtin___clear_cache.  */
+/* Emit a call to __builtin___clear_cache.  */
 
-static rtx
-expand_builtin___clear_cache (tree exp)
+void
+default_emit_call_builtin___clear_cache (rtx begin, rtx end)
 {
-  if (!targetm.code_for_clear_cache)
+  rtx callee = gen_rtx_SYMBOL_REF (Pmode,
+				   BUILTIN_ASM_NAME_PTR
+				   (BUILT_IN_CLEAR_CACHE));
+
+  emit_library_call (callee,
+		     LCT_NORMAL, VOIDmode,
+		     begin, ptr_mode,
+		     end, ptr_mode);
+}
+
+/* Emit a call to __builtin___clear_cache, unless the target specifies
+   it as do-nothing.  This function can be used by trampoline
+   finalizers to duplicate the effects of expanding a call to the
+   clear_cache builtin.  */
+
+void
+maybe_emit_call_builtin___clear_cache (rtx begin, rtx end)
+{
+  if (GET_MODE (begin) != ptr_mode || GET_MODE (end) != ptr_mode)
     {
-#ifdef CLEAR_INSN_CACHE
-      /* There is no "clear_cache" insn, and __clear_cache() in libgcc
-	 does something.  Just do the default expansion to a call to
-	 __clear_cache().  */
-      return NULL_RTX;
-#else
+      error ("both arguments to %<__builtin___clear_cache%> must be pointers");
+      return;
+    }
+
+  if (targetm.have_clear_cache ())
+    {
+      /* We have a "clear_cache" insn, and it will handle everything.  */
+      class expand_operand ops[2];
+
+      create_address_operand (&ops[0], begin);
+      create_address_operand (&ops[1], end);
+
+      if (maybe_expand_insn (targetm.code_for_clear_cache, 2, ops))
+	return;
+    }
+  else
+    {
+#ifndef CLEAR_INSN_CACHE
       /* There is no "clear_cache" insn, and __clear_cache() in libgcc
 	 does nothing.  There is no need to call it.  Do nothing.  */
-      return const0_rtx;
+      return;
 #endif /* CLEAR_INSN_CACHE */
     }
 
-  /* We have a "clear_cache" insn, and it will handle everything.  */
+  targetm.calls.emit_call_builtin___clear_cache (begin, end);
+}
+
+/* Expand a call to __builtin___clear_cache.  */
+
+static void
+expand_builtin___clear_cache (tree exp)
+{
   tree begin, end;
   rtx begin_rtx, end_rtx;
 
@@ -7082,25 +7119,16 @@ expand_builtin___clear_cache (tree exp)
   if (!validate_arglist (exp, POINTER_TYPE, POINTER_TYPE, VOID_TYPE))
     {
       error ("both arguments to %<__builtin___clear_cache%> must be pointers");
-      return const0_rtx;
+      return;
     }
 
-  if (targetm.have_clear_cache ())
-    {
-      class expand_operand ops[2];
+  begin = CALL_EXPR_ARG (exp, 0);
+  begin_rtx = expand_expr (begin, NULL_RTX, Pmode, EXPAND_NORMAL);
 
-      begin = CALL_EXPR_ARG (exp, 0);
-      begin_rtx = expand_expr (begin, NULL_RTX, Pmode, EXPAND_NORMAL);
+  end = CALL_EXPR_ARG (exp, 1);
+  end_rtx = expand_expr (end, NULL_RTX, Pmode, EXPAND_NORMAL);
 
-      end = CALL_EXPR_ARG (exp, 1);
-      end_rtx = expand_expr (end, NULL_RTX, Pmode, EXPAND_NORMAL);
-
-      create_address_operand (&ops[0], begin_rtx);
-      create_address_operand (&ops[1], end_rtx);
-      if (maybe_expand_insn (targetm.code_for_clear_cache, 2, ops))
-	return const0_rtx;
-    }
-  return const0_rtx;
+  maybe_emit_call_builtin___clear_cache (begin_rtx, end_rtx);
 }
 
 /* Given a trampoline address, make sure it satisfies TRAMPOLINE_ALIGNMENT.  */
@@ -8790,6 +8818,7 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
       && fcode != BUILT_IN_EXECLE
       && fcode != BUILT_IN_EXECVP
       && fcode != BUILT_IN_EXECVE
+      && fcode != BUILT_IN_CLEAR_CACHE
       && !ALLOCA_FUNCTION_CODE_P (fcode)
       && fcode != BUILT_IN_FREE)
     return expand_call (exp, target, ignore);
@@ -8979,10 +9008,8 @@ expand_builtin (tree exp, rtx target, rtx subtarget, machine_mode mode,
       return expand_builtin_next_arg ();
 
     case BUILT_IN_CLEAR_CACHE:
-      target = expand_builtin___clear_cache (exp);
-      if (target)
-        return target;
-      break;
+      expand_builtin___clear_cache (exp);
+      return const0_rtx;
 
     case BUILT_IN_CLASSIFY_TYPE:
       return expand_builtin_classify_type (exp);
diff --git a/gcc/builtins.h b/gcc/builtins.h
index c09f36da02b76..40aa2959e8fc5 100644
--- a/gcc/builtins.h
+++ b/gcc/builtins.h
@@ -128,6 +128,7 @@ extern tree fold_call_expr (location_t, tree, bool);
 extern tree fold_builtin_call_array (location_t, tree, tree, int, tree *);
 extern bool validate_gimple_arglist (const gcall *, ...);
 extern rtx default_expand_builtin (tree, rtx, rtx, machine_mode, int);
+extern void maybe_emit_call_builtin___clear_cache (rtx, rtx);
 extern bool fold_builtin_next_arg (tree, bool);
 extern tree do_mpc_arg2 (tree, tree, tree, int, int (*)(mpc_ptr, mpc_srcptr, mpc_srcptr, mpc_rnd_t));
 extern tree fold_call_stmt (gcall *, bool);
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index 97cb68980e975..e736656c5a9b3 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -11008,10 +11008,10 @@ aarch64_trampoline_init (rtx m_tramp, tree fndecl, rtx chain_value)
   /* XXX We should really define a "clear_cache" pattern and use
      gen_clear_cache().  */
   a_tramp = XEXP (m_tramp, 0);
-  emit_library_call (gen_rtx_SYMBOL_REF (Pmode, "__clear_cache"),
-		     LCT_NORMAL, VOIDmode, a_tramp, ptr_mode,
-		     plus_constant (ptr_mode, a_tramp, TRAMPOLINE_SIZE),
-		     ptr_mode);
+  maybe_emit_call_builtin___clear_cache (a_tramp,
+					 plus_constant (ptr_mode,
+							a_tramp,
+							TRAMPOLINE_SIZE));
 }
 
 static unsigned char
diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
index 2a7b1fb48bc4b..eabc122d5f121 100644
--- a/gcc/config/arc/arc.c
+++ b/gcc/config/arc/arc.c
@@ -4418,10 +4418,10 @@ arc_initialize_trampoline (rtx tramp, tree fndecl, rtx cxt)
 		   GEN_INT (TRAMPOLINE_SIZE), BLOCK_OP_NORMAL);
   emit_move_insn (adjust_address (tramp, SImode, 8), fnaddr);
   emit_move_insn (adjust_address (tramp, SImode, 12), cxt);
-  emit_library_call (gen_rtx_SYMBOL_REF (Pmode, "__clear_cache"),
-		     LCT_NORMAL, VOIDmode, XEXP (tramp, 0), Pmode,
-		     plus_constant (Pmode, XEXP (tramp, 0), TRAMPOLINE_SIZE),
-		     Pmode);
+  maybe_emit_call_builtin___clear_cache (XEXP (tramp, 0),
+					 plus_constant (Pmode,
+							XEXP (tramp, 0),
+							TRAMPOLINE_SIZE));
 }
 
 /* Add the given function declaration to emit code in JLI section.  */
diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c
index 5612d1e7e1805..d86c1c205d616 100644
--- a/gcc/config/arm/arm.c
+++ b/gcc/config/arm/arm.c
@@ -4170,9 +4170,10 @@ arm_trampoline_init (rtx m_tramp, tree fndecl, rtx chain_value)
     }
 
   a_tramp = XEXP (m_tramp, 0);
-  emit_library_call (gen_rtx_SYMBOL_REF (Pmode, "__clear_cache"),
-		     LCT_NORMAL, VOIDmode, a_tramp, Pmode,
-		     plus_constant (Pmode, a_tramp, TRAMPOLINE_SIZE), Pmode);
+  maybe_emit_call_builtin___clear_cache (a_tramp,
+					 plus_constant (ptr_mode,
+							a_tramp,
+							TRAMPOLINE_SIZE));
 }
 
 /* Thumb trampolines should be entered in thumb mode, so set
diff --git a/gcc/config/c6x/c6x.c b/gcc/config/c6x/c6x.c
index 9aa7ef0620c83..0ba6b1b46f26b 100644
--- a/gcc/config/c6x/c6x.c
+++ b/gcc/config/c6x/c6x.c
@@ -725,11 +725,13 @@ c6x_initialize_trampoline (rtx tramp, tree fndecl, rtx cxt)
     }
 #ifdef CLEAR_INSN_CACHE
   tramp = XEXP (tramp, 0);
-  emit_library_call (gen_rtx_SYMBOL_REF (Pmode, "__gnu_clear_cache"),
-		     LCT_NORMAL, VOIDmode, tramp, Pmode,
-		     plus_constant (Pmode, tramp, TRAMPOLINE_SIZE), Pmode);
+  maybe_emit_call_builtin___clear_cache (tramp,
+					 plus_constant (Pmode,
+							tramp,
+							TRAMPOLINE_SIZE));
 #endif
 }
+
 \f
 /* Determine whether c6x_output_mi_thunk can succeed.  */
 
diff --git a/gcc/config/csky/csky.c b/gcc/config/csky/csky.c
index 5aa233677bcc7..3b03f3f19694b 100644
--- a/gcc/config/csky/csky.c
+++ b/gcc/config/csky/csky.c
@@ -5917,9 +5917,10 @@ csky_trampoline_init (rtx m_tramp, tree fndecl, rtx chain_value)
   emit_move_insn (mem, fnaddr);
 
   a_tramp = XEXP (m_tramp, 0);
-  emit_library_call (gen_rtx_SYMBOL_REF (Pmode, "__clear_cache"),
-		     LCT_NORMAL, VOIDmode, a_tramp, Pmode,
-		     plus_constant (Pmode, a_tramp, TRAMPOLINE_SIZE), Pmode);
+  maybe_emit_call_builtin___clear_cache (a_tramp,
+					 plus_constant (Pmode,
+							a_tramp,
+							TRAMPOLINE_SIZE));
 }
 
 
diff --git a/gcc/config/m68k/linux.h b/gcc/config/m68k/linux.h
index 0d18e5ae5ac16..a01647c62107e 100644
--- a/gcc/config/m68k/linux.h
+++ b/gcc/config/m68k/linux.h
@@ -194,10 +194,10 @@ along with GCC; see the file COPYING3.  If not see
 
 #undef FINALIZE_TRAMPOLINE
 #define FINALIZE_TRAMPOLINE(TRAMP)					\
-  emit_library_call (gen_rtx_SYMBOL_REF (Pmode, "__clear_cache"),	\
-		     LCT_NORMAL, VOIDmode, TRAMP, Pmode,		\
-		     plus_constant (Pmode, TRAMP, TRAMPOLINE_SIZE), 	\
-		     Pmode);
+  maybe_emit_call_builtin___clear_cache ((TRAMP),			\
+					 plus_constant (Pmode,		\
+							(TRAMP),	\
+							TRAMPOLINE_SIZE))
 
 /* Clear the instruction cache from `beg' to `end'.  This makes an
    inline system call to SYS_cacheflush.  The arguments are as
diff --git a/gcc/config/tilegx/tilegx.c b/gcc/config/tilegx/tilegx.c
index 92a16551ac00d..142e3427c2c4c 100644
--- a/gcc/config/tilegx/tilegx.c
+++ b/gcc/config/tilegx/tilegx.c
@@ -5049,9 +5049,7 @@ tilegx_trampoline_init (rtx m_tramp, tree fndecl, rtx static_chain)
   end_addr = force_reg (Pmode, plus_constant (Pmode, XEXP (m_tramp, 0),
 					      TRAMPOLINE_SIZE));
 
-  emit_library_call (gen_rtx_SYMBOL_REF (Pmode, "__clear_cache"),
-		     LCT_NORMAL, VOIDmode, begin_addr, Pmode,
-		     end_addr, Pmode);
+  maybe_emit_call_builtin___clear_cache (begin_addr, end_addr);
 }
 
 
diff --git a/gcc/config/tilepro/tilepro.c b/gcc/config/tilepro/tilepro.c
index 540c6356c08fb..3990194cadef9 100644
--- a/gcc/config/tilepro/tilepro.c
+++ b/gcc/config/tilepro/tilepro.c
@@ -4458,9 +4458,7 @@ tilepro_trampoline_init (rtx m_tramp, tree fndecl, rtx static_chain)
   end_addr = force_reg (Pmode, plus_constant (Pmode, XEXP (m_tramp, 0),
 					      TRAMPOLINE_SIZE));
 
-  emit_library_call (gen_rtx_SYMBOL_REF (Pmode, "__clear_cache"),
-		     LCT_NORMAL, VOIDmode, begin_addr, Pmode,
-		     end_addr, Pmode);
+  maybe_emit_call_builtin___clear_cache (begin_addr, end_addr);
 }
 
 
diff --git a/gcc/config/vxworks.c b/gcc/config/vxworks.c
index ca0f5de12a742..b67d21d07c635 100644
--- a/gcc/config/vxworks.c
+++ b/gcc/config/vxworks.c
@@ -27,6 +27,9 @@ along with GCC; see the file COPYING3.  If not see
 #include "diagnostic-core.h"
 #include "output.h"
 #include "fold-const.h"
+#include "rtl.h"
+#include "memmodel.h"
+#include "optabs.h"
 
 #if !HAVE_INITFINI_ARRAY_SUPPORT
 /* Like default_named_section_asm_out_constructor, except that even
@@ -169,4 +172,25 @@ vxworks_override_options (void)
 
   if (!global_options_set.x_dwarf_version)
     dwarf_version = VXWORKS_DWARF_VERSION_DEFAULT;
+
+}
+
+/* We don't want to use library symbol __clear_cache on SR0640.  Avoid
+   it and issue a direct call to cacheTextUpdate.  It takes a size_t
+   length rather than the END address, so we have to compute it.  */
+
+void
+vxworks_emit_call_builtin___clear_cache (rtx begin, rtx end)
+{
+  /* STATUS cacheTextUpdate (void *, size_t); */
+  rtx callee = gen_rtx_SYMBOL_REF (Pmode, "cacheTextUpdate");
+
+  enum machine_mode size_mode = TYPE_MODE (sizetype);
+
+  rtx len = simplify_gen_binary (MINUS, size_mode, end, begin);
+
+  emit_library_call (callee,
+		     LCT_NORMAL, VOIDmode,
+		     begin, ptr_mode,
+		     len, size_mode);
 }
diff --git a/gcc/config/vxworks.h b/gcc/config/vxworks.h
index e2ce22bec8b67..cd4313982fa38 100644
--- a/gcc/config/vxworks.h
+++ b/gcc/config/vxworks.h
@@ -282,10 +282,13 @@ extern void vxworks_asm_out_destructor (rtx symbol, int priority);
 /* The diab linker does not handle .gnu_attribute sections.  */
 #undef HAVE_AS_GNU_ATTRIBUTE
 
-/* We provide our own version of __clear_cache in libgcc, using a separate C
-   file to facilitate #inclusion of VxWorks header files.  */
-#undef CLEAR_INSN_CACHE
-#define CLEAR_INSN_CACHE 1
+/* We call vxworks's cacheTextUpdate instead of CLEAR_INSN_CACHE if
+   needed.  We don't want to force a call on targets that don't define
+   cache-clearing insns nor CLEAR_INSN_CACHE.  */
+#undef TARGET_EMIT_CALL_BUILTIN___CLEAR_CACHE
+#define TARGET_EMIT_CALL_BUILTIN___CLEAR_CACHE \
+  vxworks_emit_call_builtin___clear_cache
+extern void vxworks_emit_call_builtin___clear_cache (rtx begin, rtx end);
 
 /* Default dwarf control values, for non-gdb debuggers that come with
    VxWorks.  */
diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi
index 833320ba7bfbe..55ed7ea382370 100644
--- a/gcc/doc/tm.texi
+++ b/gcc/doc/tm.texi
@@ -5457,11 +5457,24 @@ Note that the block move need only cover the constant parts of the
 trampoline.  If the target isolates the variable parts of the trampoline
 to the end, not all @code{TRAMPOLINE_SIZE} bytes need be copied.
 
-If the target requires any other actions, such as flushing caches or
+If the target requires any other actions, such as flushing caches
+(possibly calling function maybe_emit_call_builtin___clear_cache) or
 enabling stack execution, these actions should be performed after
 initializing the trampoline proper.
 @end deftypefn
 
+@deftypefn {Target Hook} void TARGET_EMIT_CALL_BUILTIN___CLEAR_CACHE (rtx @var{begin}, rtx @var{end})
+On targets that do not define a @code{clear_cache} insn expander,
+but that define the @code{CLEAR_CACHE_INSN} macro,
+maybe_emit_call_builtin___clear_cache relies on this target hook
+to clear an address range in the instruction cache.
+
+The default implementation calls the @code{__clear_cache} builtin,
+taking the assembler name from the builtin declaration.  Overriding
+definitions may call alternate functions, with alternate calling
+conventions, or emit alternate RTX to perform the job.
+@end deftypefn
+
 @deftypefn {Target Hook} rtx TARGET_TRAMPOLINE_ADJUST_ADDRESS (rtx @var{addr})
 This hook should perform any machine-specific adjustment in
 the address of the trampoline.  Its argument contains the address of the
@@ -5490,7 +5503,7 @@ the following macro.
 If defined, expands to a C expression clearing the @emph{instruction
 cache} in the specified interval.  The definition of this macro would
 typically be a series of @code{asm} statements.  Both @var{beg} and
-@var{end} are both pointer expressions.
+@var{end} are pointer expressions.
 @end defmac
 
 To use a standard subroutine, define the following macro.  In addition,
diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in
index 58109be36932d..3df4b24846281 100644
--- a/gcc/doc/tm.texi.in
+++ b/gcc/doc/tm.texi.in
@@ -3877,6 +3877,8 @@ is used for aligning trampolines.
 
 @hook TARGET_TRAMPOLINE_INIT
 
+@hook TARGET_EMIT_CALL_BUILTIN___CLEAR_CACHE
+
 @hook TARGET_TRAMPOLINE_ADJUST_ADDRESS
 
 Implementing trampolines is difficult on many machines because they have
@@ -3897,7 +3899,7 @@ the following macro.
 If defined, expands to a C expression clearing the @emph{instruction
 cache} in the specified interval.  The definition of this macro would
 typically be a series of @code{asm} statements.  Both @var{beg} and
-@var{end} are both pointer expressions.
+@var{end} are pointer expressions.
 @end defmac
 
 To use a standard subroutine, define the following macro.  In addition,
diff --git a/gcc/target.def b/gcc/target.def
index b916635be1816..f29fbce7963ed 100644
--- a/gcc/target.def
+++ b/gcc/target.def
@@ -5153,12 +5153,28 @@ Note that the block move need only cover the constant parts of the\n\
 trampoline.  If the target isolates the variable parts of the trampoline\n\
 to the end, not all @code{TRAMPOLINE_SIZE} bytes need be copied.\n\
 \n\
-If the target requires any other actions, such as flushing caches or\n\
+If the target requires any other actions, such as flushing caches\n\
+(possibly calling function maybe_emit_call_builtin___clear_cache) or\n\
 enabling stack execution, these actions should be performed after\n\
 initializing the trampoline proper.",
  void, (rtx m_tramp, tree fndecl, rtx static_chain),
  default_trampoline_init)
 
+/* Emit a call to a function to clear the instruction cache.  */
+DEFHOOK
+(emit_call_builtin___clear_cache,
+ "On targets that do not define a @code{clear_cache} insn expander,\n\
+but that define the @code{CLEAR_CACHE_INSN} macro,\n\
+maybe_emit_call_builtin___clear_cache relies on this target hook\n\
+to clear an address range in the instruction cache.\n\
+\n\
+The default implementation calls the @code{__clear_cache} builtin,\n\
+taking the assembler name from the builtin declaration.  Overriding\n\
+definitions may call alternate functions, with alternate calling\n\
+conventions, or emit alternate RTX to perform the job.",
+ void, (rtx begin, rtx end),
+ default_emit_call_builtin___clear_cache)
+
 /* Adjust the address of the trampoline in a target-specific way.  */
 DEFHOOK
 (trampoline_adjust_address,
diff --git a/gcc/targhooks.h b/gcc/targhooks.h
index e0a925fa2bee2..4ff33e2de124b 100644
--- a/gcc/targhooks.h
+++ b/gcc/targhooks.h
@@ -166,6 +166,7 @@ extern bool default_function_value_regno_p (const unsigned int);
 extern rtx default_internal_arg_pointer (void);
 extern rtx default_static_chain (const_tree, bool);
 extern void default_trampoline_init (rtx, tree, rtx);
+extern void default_emit_call_builtin___clear_cache (rtx, rtx);
 extern poly_int64 default_return_pops_args (tree, tree, poly_int64);
 extern reg_class_t default_ira_change_pseudo_allocno_class (int, reg_class_t,
 							    reg_class_t);
diff --git a/gcc/tree.h b/gcc/tree.h
index 684be10b440a7..d69f8bad1d015 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -5579,6 +5579,13 @@ is_lang_specific (const_tree t)
 #define BUILTIN_VALID_P(FNCODE) \
   (IN_RANGE ((int)FNCODE, ((int)BUILT_IN_NONE) + 1, ((int) END_BUILTINS) - 1))
 
+/* Obtain a pointer to the identifier string holding the asm name for
+   BUILTIN, a BUILT_IN code.  This is handy if the target
+   mangles/overrides the function name that implements the
+   builtin.  */
+#define BUILTIN_ASM_NAME_PTR(BUILTIN) \
+  (IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (builtin_decl_explicit (BUILTIN))))
+
 /* Return the tree node for an explicit standard builtin function or NULL.  */
 static inline tree
 builtin_decl_explicit (enum built_in_function fncode)
diff --git a/libgcc/config/t-vxworks b/libgcc/config/t-vxworks
index 02e2efa0eb7d7..b4bb85bff08be 100644
--- a/libgcc/config/t-vxworks
+++ b/libgcc/config/t-vxworks
@@ -4,7 +4,6 @@ LIBGCC2_DEBUG_CFLAGS =
 # We provide our own implementation for __clear_cache, using a
 # VxWorks specific entry point.
 LIB2FUNCS_EXCLUDE += _clear_cache
-LIB2ADD += $(srcdir)/config/vxcache.c
 
 # This ensures that the correct target headers are used; some VxWorks
 # system headers have names that collide with GCC's internal (host)
diff --git a/libgcc/config/t-vxworks7 b/libgcc/config/t-vxworks7
index 20c72f490dd3c..6ddd3e84f3309 100644
--- a/libgcc/config/t-vxworks7
+++ b/libgcc/config/t-vxworks7
@@ -4,7 +4,6 @@ LIBGCC2_DEBUG_CFLAGS =
 # We provide our own implementation for __clear_cache, using a
 # VxWorks specific entry point.
 LIB2FUNCS_EXCLUDE += _clear_cache
-LIB2ADD += $(srcdir)/config/vxcache.c
 
 # This ensures that the correct target headers are used; some VxWorks
 # system headers have names that collide with GCC's internal (host)
diff --git a/libgcc/config/vxcache.c b/libgcc/config/vxcache.c
deleted file mode 100644
index e25e0cce0a4c2..0000000000000
--- a/libgcc/config/vxcache.c
+++ /dev/null
@@ -1,35 +0,0 @@
-/* Copyright (C) 2018-2020 Free Software Foundation, Inc.
-   Contributed by Alexandre Oliva <oliva@adacore.com>
-
-This file is part of GCC.
-
-GCC is free software; you can redistribute it and/or modify it under
-the terms of the GNU General Public License as published by the Free
-Software Foundation; either version 3, or (at your option) any later
-version.
-
-GCC is distributed in the hope that it will be useful, but WITHOUT ANY
-WARRANTY; without even the implied warranty of MERCHANTABILITY or
-FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
-for more details.
-
-Under Section 7 of GPL version 3, you are granted additional
-permissions described in the GCC Runtime Library Exception, version
-3.1, as published by the Free Software Foundation.
-
-You should have received a copy of the GNU General Public License and
-a copy of the GCC Runtime Library Exception along with this program;
-see the files COPYING3 and COPYING.RUNTIME respectively.  If not, see
-<http://www.gnu.org/licenses/>.  */
-
-/* Instruction cache invalidation routine using VxWorks' cacheLib.  */
-
-#include <vxWorks.h>
-#include <cacheLib.h>
-
-void
-__clear_cache (char *beg __attribute__((__unused__)),
-	       char *end __attribute__((__unused__)))
-{
-  cacheTextUpdate (beg, end - beg);
-}


-- 
Alexandre Oliva, happy hacker
https://FSFLA.org/blogs/lxo/
Free Software Activist
GNU Toolchain Engineer

Re: introduce overridable clear_cache emitter

[Olivier Hainque]

Hi Alex,

Thanks for your proposal on this!

> On 11 Nov 2020, at 03:35, Alexandre Oliva <oliva@adacore.com> wrote:
> 
> This patch introduces maybe_emit_call_builtin___clear_cache for the
> builtin expander machinery and the trampoline initializers to use to
> clear the instruction cache, removing a source of inconsistencies and
> subtle errors in low-level machinery.
> 
> I've adjusted all trampoline_init implementations that used to issue
> explicit calls to __clear_cache or similar to use this new primitive.
> 
> Specifically on vxworks targets, we needed to drop the __clear_cache
> symbol in libgcc, for reasons related with linking that I didn't need
> to understand, and we wanted to call cacheTextUpdate directly, despite
> the different calling conventions: the second argument is a length
> rather than the end address.

The initial VxWorks issue is essentially that recent (llvm based)
environments know __clear_cache as a builtin and refuse to statically
link modules that contain an exposed symbol with that name.

As VxWorks (indeed) has a public API for the kind of insn
cache clearing operations we need, it just seems natural to arrange
to call it directly instead of going through an intermediate wrapper
in libgcc. This is a bit more efficient and removes a dependency
on libgcc, a good thing in an environment where there are multiple
ways to link things together.

It also seems like a positive thing to tighten the
connection/consistency between what trampolines and __builtin_clear_cache
do, as they're supposed to be used for similar purposes AFAICT.

> So I introduced a target hook to enable target OS-level overriding of
> builtin __clear_cache call emission, retaining nearly (*) the same
> logic to govern the decision on whether to emit a call (or nothing, or
> a machine-dependent insn) but enabling a call to a target
> system-defined function with different calling conventions to be
> issued, without having to modify .md files of the various
> architectures supported by the target system to introduce or modify
> clear_cache insns.

No strong feeling on the means here, so I'll defer to other
maintainers on the approach.

IIUC, the patch intends not to change the behavior of back-ends on
the trampoline init paths, which seems like a key property to me.

Olivier

Re: introduce overridable clear_cache emitter

[Jeff Law]

On 11/10/20 7:35 PM, Alexandre Oliva wrote:
> This patch introduces maybe_emit_call_builtin___clear_cache for the
> builtin expander machinery and the trampoline initializers to use to
> clear the instruction cache, removing a source of inconsistencies and
> subtle errors in low-level machinery.
>
> I've adjusted all trampoline_init implementations that used to issue
> explicit calls to __clear_cache or similar to use this new primitive.
>
>
> Specifically on vxworks targets, we needed to drop the __clear_cache
> symbol in libgcc, for reasons related with linking that I didn't need
> to understand, and we wanted to call cacheTextUpdate directly, despite
> the different calling conventions: the second argument is a length
> rather than the end address.
>
> So I introduced a target hook to enable target OS-level overriding of
> builtin __clear_cache call emission, retaining nearly (*) the same
> logic to govern the decision on whether to emit a call (or nothing, or
> a machine-dependent insn) but enabling a call to a target
> system-defined function with different calling conventions to be
> issued, without having to modify .md files of the various
> architectures supported by the target system to introduce or modify
> clear_cache insns.
>
> (*) I write "nearly" mainly because, when not optimizing, we'd issue a
> call regardless, but since the call may now be overridden, I added it
> to the set of builtins that are not directly turned into calls when
> not optimizing, following the normal expansion path instead.  It
> wouldn't be hard to skip the emission of cache-clearing insns when not
> optimizing, but it didn't seem very important, especially for the new
> uses from trampoline init.
>
>     Another difference that might be relevant is that now we expand
> the begin and end arguments unconditionally.  This might make a
> difference if they have side effects.  That's prettty much impossible
> at expand time, but I thought I'd mention it.
>
>
> I have NOT modified targets that did not issue cache-clearing calls in
> trampoline init to use the new clear_cache-calling infrastructure even
> if it would expand to nothing.  I have considered doing so, to have
> __builtin___clear_cache and trampoline init call cacheTextUpdate on
> all vxworks targets, but decided not to, since on targets that don't
> do any cache clearing, cacheTextUpdate ought to be a no-op, even
> though rs6000 seems to use icbi and dcbf instructions in the function
> called to initialize a trampoline, but AFAICT not in the __clear_cache
> builtin.  Hopefully target maintainers will have a look and take
> advantage of this new piece of infrastructure to remove such
> (apparent?) inconsistencies.  Not rs6000 and other that call asm-coded
> trampoline setup instructions, for sure, but they might wish to
> introduce a CLEAR_INSN_CACHE macro or a clear_cache expander if they
> don't have one.
>
>
> This was regstrapped on x86_64-linux-gnu, and a trivial backport was
> tested on multiple vxworks targets.  Ok to install?
>
>
> for  gcc/ChangeLog
>
> 	* builtins.c (default_emit_call_builtin___clear_cache): New.
> 	(maybe_emit_call_builtin___clear_cache): New.
> 	(expand_builtin___clear_cache): Split into the above.
> 	(expand_builtin): Do not issue clear_cache call any more.
> 	* builtins.h (maybe_emit_call_builtin___clear_cache): Declare.
> 	* config/aarch64/aarch64.c (aarch64_trampoline_init): Use
> 	maybe_emit_call_builtin___clear_cache.
> 	* config/arc/arc.c (arc_trampoline_init): Likewise.
> 	* config/arm/arm.c (arm_trampoline_init): Likewise.
> 	* config/c6x/c6x.c (c6x_initialize_trampoline): Likewise.
> 	* config/csky/csky.c (csky_trampoline_init): Likewise.
> 	* config/m68k/linux.h (FInALIZE_TRAMPOLINE): Likewise.
> 	* config/tilegx/tilegx.c (tilegx_trampoline_init): Likewise.
> 	* config/tilepro/tilepro.c (tilepro_trampoline_init): Ditto.
> 	* config/vxworks.c: Include rtl.h, memmodel.h, and optabs.h.
> 	(vxworks_emit_call_builtin___clear_cache): New.
> 	* config/vxworks.h (CLEAR_INSN_CACHE): Drop.
> 	(TARGET_EMIT_CALL_BUILTIN___CLEAR_CACHE): Define.
> 	* target.def (trampoline_init): In the documentation, refer to
> 	maybe_emit_call_builtin___clear_cache.
> 	(emit_call_builtin___clear_cache): New.
> 	* doc/tm.texi.in: Add new hook point.
> 	(CLEAR_CACHE_INSN): Remove duplicate 'both'.
> 	* doc/tm.texi: Rebuilt.
> 	* targhooks.h (default_meit_call_builtin___clear_cache):
> 	Declare.
> 	* tree.h (BUILTIN_ASM_NAME_PTR): New.
>
> for  libgcc/ChangeLog
>
> 	* config/t-vxworks (LIB2ADD): Drop.
> 	* config/t-vxworks7 (LIB2ADD): Likewise.
> 	* config/vxcache.c: Remove.
OK
jeff

Re: introduce overridable clear_cache emitter

[Christophe Lyon]

Hi Alexandre,

On Wed, 2 Dec 2020 at 19:23, Jeff Law via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> On 11/10/20 7:35 PM, Alexandre Oliva wrote:
> > This patch introduces maybe_emit_call_builtin___clear_cache for the
> > builtin expander machinery and the trampoline initializers to use to
> > clear the instruction cache, removing a source of inconsistencies and
> > subtle errors in low-level machinery.
> >
> > I've adjusted all trampoline_init implementations that used to issue
> > explicit calls to __clear_cache or similar to use this new primitive.
> >
> >
> > Specifically on vxworks targets, we needed to drop the __clear_cache
> > symbol in libgcc, for reasons related with linking that I didn't need
> > to understand, and we wanted to call cacheTextUpdate directly, despite
> > the different calling conventions: the second argument is a length
> > rather than the end address.
> >
> > So I introduced a target hook to enable target OS-level overriding of
> > builtin __clear_cache call emission, retaining nearly (*) the same
> > logic to govern the decision on whether to emit a call (or nothing, or
> > a machine-dependent insn) but enabling a call to a target
> > system-defined function with different calling conventions to be
> > issued, without having to modify .md files of the various
> > architectures supported by the target system to introduce or modify
> > clear_cache insns.
> >
> > (*) I write "nearly" mainly because, when not optimizing, we'd issue a
> > call regardless, but since the call may now be overridden, I added it
> > to the set of builtins that are not directly turned into calls when
> > not optimizing, following the normal expansion path instead.  It
> > wouldn't be hard to skip the emission of cache-clearing insns when not
> > optimizing, but it didn't seem very important, especially for the new
> > uses from trampoline init.
> >
> >     Another difference that might be relevant is that now we expand
> > the begin and end arguments unconditionally.  This might make a
> > difference if they have side effects.  That's prettty much impossible
> > at expand time, but I thought I'd mention it.
> >
> >
> > I have NOT modified targets that did not issue cache-clearing calls in
> > trampoline init to use the new clear_cache-calling infrastructure even
> > if it would expand to nothing.  I have considered doing so, to have
> > __builtin___clear_cache and trampoline init call cacheTextUpdate on
> > all vxworks targets, but decided not to, since on targets that don't
> > do any cache clearing, cacheTextUpdate ought to be a no-op, even
> > though rs6000 seems to use icbi and dcbf instructions in the function
> > called to initialize a trampoline, but AFAICT not in the __clear_cache
> > builtin.  Hopefully target maintainers will have a look and take
> > advantage of this new piece of infrastructure to remove such
> > (apparent?) inconsistencies.  Not rs6000 and other that call asm-coded
> > trampoline setup instructions, for sure, but they might wish to
> > introduce a CLEAR_INSN_CACHE macro or a clear_cache expander if they
> > don't have one.
> >
> >
> > This was regstrapped on x86_64-linux-gnu, and a trivial backport was
> > tested on multiple vxworks targets.  Ok to install?
> >
> >
> > for  gcc/ChangeLog
> >
> >       * builtins.c (default_emit_call_builtin___clear_cache): New.
> >       (maybe_emit_call_builtin___clear_cache): New.
> >       (expand_builtin___clear_cache): Split into the above.
> >       (expand_builtin): Do not issue clear_cache call any more.
> >       * builtins.h (maybe_emit_call_builtin___clear_cache): Declare.
> >       * config/aarch64/aarch64.c (aarch64_trampoline_init): Use
> >       maybe_emit_call_builtin___clear_cache.
> >       * config/arc/arc.c (arc_trampoline_init): Likewise.
> >       * config/arm/arm.c (arm_trampoline_init): Likewise.
> >       * config/c6x/c6x.c (c6x_initialize_trampoline): Likewise.
> >       * config/csky/csky.c (csky_trampoline_init): Likewise.
> >       * config/m68k/linux.h (FInALIZE_TRAMPOLINE): Likewise.
> >       * config/tilegx/tilegx.c (tilegx_trampoline_init): Likewise.
> >       * config/tilepro/tilepro.c (tilepro_trampoline_init): Ditto.
> >       * config/vxworks.c: Include rtl.h, memmodel.h, and optabs.h.
> >       (vxworks_emit_call_builtin___clear_cache): New.
> >       * config/vxworks.h (CLEAR_INSN_CACHE): Drop.
> >       (TARGET_EMIT_CALL_BUILTIN___CLEAR_CACHE): Define.
> >       * target.def (trampoline_init): In the documentation, refer to
> >       maybe_emit_call_builtin___clear_cache.
> >       (emit_call_builtin___clear_cache): New.
> >       * doc/tm.texi.in: Add new hook point.
> >       (CLEAR_CACHE_INSN): Remove duplicate 'both'.
> >       * doc/tm.texi: Rebuilt.
> >       * targhooks.h (default_meit_call_builtin___clear_cache):
> >       Declare.
> >       * tree.h (BUILTIN_ASM_NAME_PTR): New.
> >
> > for  libgcc/ChangeLog
> >
> >       * config/t-vxworks (LIB2ADD): Drop.
> >       * config/t-vxworks7 (LIB2ADD): Likewise.
> >       * config/vxcache.c: Remove.

This patches causes a lot of regressions in fortran on arm and aarch64,
see: https://people.linaro.org/~christophe.lyon/cross-validation/gcc/trunk/r11-5692-gc05ece92c6153289fd6055e31e791e59b8ac4121/report-build-info.html
(click on the red REGRESSED to get a summary of the regressions, and
"log" next to it to download the
corresponding gfortran.log if you need it)

Can you check/fix them?

Thanks

Christophe

> OK
> jeff
>

Re: introduce overridable clear_cache emitter

[Alexandre Oliva]

On Dec  3, 2020, Christophe Lyon <christophe.lyon@linaro.org> wrote:

> This patches causes a lot of regressions in fortran on arm and aarch64,

Thanks, on it.

-- 
Alexandre Oliva, happy hacker  https://FSFLA.org/blogs/lxo/
   Free Software Activist         GNU Toolchain Engineer
        Vim, Vi, Voltei pro Emacs -- GNUlius Caesar

Re: introduce overridable clear_cache emitter

[Andreas Schwab]

../../../../libffi/src/aarch64/ffi.c: In function 'ffi_prep_closure_loc':
../../../../libffi/src/aarch64/ffi.c:67:3: error: both arguments to '__builtin___clear_cache' must be pointers
   67 |   __builtin___clear_cache (start, end);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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: introduce overridable clear_cache emitter

[Andreas Schwab]

On Dez 03 2020, Andreas Schwab wrote:

> ../../../../libffi/src/aarch64/ffi.c: In function 'ffi_prep_closure_loc':
> ../../../../libffi/src/aarch64/ffi.c:67:3: error: both arguments to '__builtin___clear_cache' must be pointers
>    67 |   __builtin___clear_cache (start, end);
>       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This happens when compiling with -mabi=ilp32.

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: introduce overridable clear_cache emitter

[Alexandre Oliva]

On Dec  3, 2020, Christophe Lyon <christophe.lyon@linaro.org> wrote:

> This patches causes a lot of regressions in fortran on arm and aarch64,

On Dec  3, 2020, Andreas Schwab <schwab@linux-m68k.org> wrote:

> On Dez 03 2020, Andreas Schwab wrote:

>> ../../../../libffi/src/aarch64/ffi.c: In function 'ffi_prep_closure_loc':
>> ../../../../libffi/src/aarch64/ffi.c:67:3: error: both arguments to '__builtin___clear_cache' must be pointers
>> 67 |   __builtin___clear_cache (start, end);
>> |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

> This happens when compiling with -mabi=ilp32.

Thank you both.  Here's the patch I'm testing to fix both issues.
Ok to install?


fix __builtin___clear_cache overrider fallout

From: Alexandre Oliva <oliva@adacore.com>

Machines that had CLEAR_CACHE_INSN and that would thus issue calls to
__clear_cache with the default call expander, would fail on languages
that did not set up the __clear_cache builtin.  This patch arranges
for all languages to set up this builtin.

Machines or multilibs that had ptr_mode != Pmode, such as aarch64 with
-mabi=ilp32, would fail the RTL mode test of the arguments passed to
__clear_cache, because we'd insist on ptr_mode.  This patch arranges for
Pmode to be accepted as well.


for  gcc/ChangeLog

	* tree.c (build_common_builtin_nodes): Declare
	__builtin___clear_cache for all languages.
	* builtins.c (maybe_emit_call_builtin___clear_cache): Accept
	Pmode arguments.
---
 gcc/builtins.c |    3 ++-
 gcc/tree.c     |    6 ++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/gcc/builtins.c b/gcc/builtins.c
index ecc12e69c1466..cd30de8bfb035 100644
--- a/gcc/builtins.c
+++ b/gcc/builtins.c
@@ -7793,7 +7793,8 @@ default_emit_call_builtin___clear_cache (rtx begin, rtx end)
 void
 maybe_emit_call_builtin___clear_cache (rtx begin, rtx end)
 {
-  if (GET_MODE (begin) != ptr_mode || GET_MODE (end) != ptr_mode)
+  if ((GET_MODE (begin) != ptr_mode && GET_MODE (begin) != Pmode)
+      || (GET_MODE (end) != ptr_mode && GET_MODE (end) != Pmode))
     {
       error ("both arguments to %<__builtin___clear_cache%> must be pointers");
       return;
diff --git a/gcc/tree.c b/gcc/tree.c
index 52a145dd01819..72311005f57b2 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -10733,6 +10733,12 @@ build_common_builtin_nodes (void)
 
   ftype = build_function_type_list (void_type_node,
 				    ptr_type_node, ptr_type_node, NULL_TREE);
+  if (!builtin_decl_explicit_p (BUILT_IN_CLEAR_CACHE))
+    local_define_builtin ("__builtin___clear_cache", ftype,
+			  BUILT_IN_CLEAR_CACHE,
+			  "__builtin___clear_cache",
+			  ECF_NOTHROW);
+
   local_define_builtin ("__builtin_nonlocal_goto", ftype,
 			BUILT_IN_NONLOCAL_GOTO,
 			"__builtin_nonlocal_goto",


-- 
Alexandre Oliva, happy hacker  https://FSFLA.org/blogs/lxo/
   Free Software Activist         GNU Toolchain Engineer
        Vim, Vi, Voltei pro Emacs -- GNUlius Caesar

Re: introduce overridable clear_cache emitter

[Jeff Law]

On 12/3/20 7:08 AM, Alexandre Oliva wrote:
> On Dec  3, 2020, Christophe Lyon <christophe.lyon@linaro.org> wrote:
>
>> This patches causes a lot of regressions in fortran on arm and aarch64,
> On Dec  3, 2020, Andreas Schwab <schwab@linux-m68k.org> wrote:
>
>> On Dez 03 2020, Andreas Schwab wrote:
>>> ../../../../libffi/src/aarch64/ffi.c: In function 'ffi_prep_closure_loc':
>>> ../../../../libffi/src/aarch64/ffi.c:67:3: error: both arguments to '__builtin___clear_cache' must be pointers
>>> 67 |   __builtin___clear_cache (start, end);
>>> |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> This happens when compiling with -mabi=ilp32.
> Thank you both.  Here's the patch I'm testing to fix both issues.
> Ok to install?
>
>
> fix __builtin___clear_cache overrider fallout
>
> From: Alexandre Oliva <oliva@adacore.com>
>
> Machines that had CLEAR_CACHE_INSN and that would thus issue calls to
> __clear_cache with the default call expander, would fail on languages
> that did not set up the __clear_cache builtin.  This patch arranges
> for all languages to set up this builtin.
>
> Machines or multilibs that had ptr_mode != Pmode, such as aarch64 with
> -mabi=ilp32, would fail the RTL mode test of the arguments passed to
> __clear_cache, because we'd insist on ptr_mode.  This patch arranges for
> Pmode to be accepted as well.
>
>
> for  gcc/ChangeLog
>
> 	* tree.c (build_common_builtin_nodes): Declare
> 	__builtin___clear_cache for all languages.
> 	* builtins.c (maybe_emit_call_builtin___clear_cache): Accept
> 	Pmode arguments.
OK
jeff

Re: introduce overridable clear_cache emitter

[Andreas Schwab]

../../../../libffi/src/aarch64/ffi.c: In function 'ffi_prep_closure_loc':
../../../../libffi/src/aarch64/ffi.c:67:3: internal compiler error: in emit_library_call_value_1, at calls.c:5300
   67 |   __builtin___clear_cache (start, end);
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

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: introduce overridable clear_cache emitter

[Alexandre Oliva]

On Dec  5, 2020, Andreas Schwab <schwab@linux-m68k.org> wrote:

> ../../../../libffi/src/aarch64/ffi.c: In function 'ffi_prep_closure_loc':
> ../../../../libffi/src/aarch64/ffi.c:67:3: internal compiler error: in emit_library_call_value_1, at calls.c:5300
>    67 |   __builtin___clear_cache (start, end);
>       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Is this still aarch64-linux-gnu -mabi=ilp32?  I'm afraid I couldn't
duplicate this error using a cross compiler (without binutils, but with
HAVE_AS_MABI_OPTION forced enabled), and many variants of a manually
minimized ffi.c (to build without libc):

static inline void
ffi_clear_cache (void *start, void *end)
{
  __builtin___clear_cache (start, end);
}

#define FFI_TRAMPOLINE_SIZE 24

typedef struct closure {
  char tramp[FFI_TRAMPOLINE_SIZE / sizeof (long)];
} ffi_closure;

void
ffi_prep_closure_loc (ffi_closure *closure)
{
  static const unsigned char trampoline[16] = {
    0x90, 0x00, 0x00, 0x58,     /* ldr  x16, tramp+16   */
    0xf1, 0xff, 0xff, 0x10,     /* adr  x17, tramp+0    */
    0x00, 0x02, 0x1f, 0xd6      /* br   x16             */
  };
  char *tramp = closure->tramp;
  __builtin_memcpy(tramp, trampoline, sizeof (trampoline));
  ffi_clear_cache(tramp, tramp + FFI_TRAMPOLINE_SIZE);
}


Once you confirm command line and target, I'll look into cross-building
a full toolchain, or using a machine from the compile farm.

Thanks,

-- 
Alexandre Oliva, happy hacker  https://FSFLA.org/blogs/lxo/
   Free Software Activist         GNU Toolchain Engineer
        Vim, Vi, Voltei pro Emacs -- GNUlius Caesar

Re: introduce overridable clear_cache emitter

[Jakub Jelinek]

On Sat, Dec 05, 2020 at 06:01:59PM -0300, Alexandre Oliva wrote:
> On Dec  5, 2020, Andreas Schwab <schwab@linux-m68k.org> wrote:
> 
> > ../../../../libffi/src/aarch64/ffi.c: In function 'ffi_prep_closure_loc':
> > ../../../../libffi/src/aarch64/ffi.c:67:3: internal compiler error: in emit_library_call_value_1, at calls.c:5300
> >    67 |   __builtin___clear_cache (start, end);
> >       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> 
> Is this still aarch64-linux-gnu -mabi=ilp32?  I'm afraid I couldn't
> duplicate this error using a cross compiler (without binutils, but with
> HAVE_AS_MABI_OPTION forced enabled), and many variants of a manually
> minimized ffi.c (to build without libc):

See PR98147, I've put there an untested patch, but I have no way to test it.

	Jakub

Re: introduce overridable clear_cache emitter

[Alexandre Oliva]

On Dec  5, 2020, Jakub Jelinek <jakub@redhat.com> wrote:

> On Sat, Dec 05, 2020 at 06:01:59PM -0300, Alexandre Oliva wrote:
>> On Dec  5, 2020, Andreas Schwab <schwab@linux-m68k.org> wrote:
>> 
>> > ../../../../libffi/src/aarch64/ffi.c: In function 'ffi_prep_closure_loc':
>> > ../../../../libffi/src/aarch64/ffi.c:67:3: internal compiler error: in emit_library_call_value_1, at calls.c:5300
>> >    67 |   __builtin___clear_cache (start, end);
>> >       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> 
>> Is this still aarch64-linux-gnu -mabi=ilp32?  I'm afraid I couldn't
>> duplicate this error using a cross compiler (without binutils, but with
>> HAVE_AS_MABI_OPTION forced enabled), and many variants of a manually
>> minimized ffi.c (to build without libc):

> See PR98147, I've put there an untested patch, but I have no way to test it.

Thanks for the fix.  I can't imagine why that wouldn't have been hit in
my reduced-build scenario, but once I saw your patch, it was pretty
obvious that that was it, and I haven't investigated any further.

-- 
Alexandre Oliva, happy hacker  https://FSFLA.org/blogs/lxo/
   Free Software Activist         GNU Toolchain Engineer
        Vim, Vi, Voltei pro Emacs -- GNUlius Caesar

Re: introduce overridable clear_cache emitter

[Alexandre Oliva]

On Dec  3, 2020, Alexandre Oliva <oliva@adacore.com> wrote:

> +    local_define_builtin ("__builtin___clear_cache", ftype,
> +			  BUILT_IN_CLEAR_CACHE,
> +			  "__builtin___clear_cache",
> +			  ECF_NOTHROW);

Ugh, so that somehow worked for aarch64-linux-gnu-gfortran, but the
aarch64-elf Ada compiler started issuing calls to an undefined
__builtin___clear_cache symbol.

The second string actual passed to local_define_builtin binds to formal
libname, which explains at least the new problem.  I've so far
restrained my curiosity as to why it wasn't a problem on
aarch64-linux-gnu-gfortran.  I'm checking it in as obvious, so far
lightly tested on x86_64-linux-gnu and -x-aarch64-elf.


drop __builtin_ from __clear_cache libname

From: Alexandre Oliva <oliva@adacore.com>

I made a cut&pasto in my previous patch for tree.c, causing platforms
that have CLEAR_INSN_CACHE defined, and none of the internal
__clear_cache expansion overriders, to issue calls to symbols named
__builtin___clear_cache rather than __clear_cache, on languages other
than those in the C family.  Oops.

This patch removes __builtin_ from the string used as the libname for
__buuiltin___clear_cache.


for  gcc/ChangeLog

	* tree.c (build_common_builtin_nodes): Drop __builtin_ from
	__clear_cache libname.
---
 gcc/tree.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/gcc/tree.c b/gcc/tree.c
index 72311005f57b2..4eb365205e3bd 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -10736,7 +10736,7 @@ build_common_builtin_nodes (void)
   if (!builtin_decl_explicit_p (BUILT_IN_CLEAR_CACHE))
     local_define_builtin ("__builtin___clear_cache", ftype,
 			  BUILT_IN_CLEAR_CACHE,
-			  "__builtin___clear_cache",
+			  "__clear_cache",
 			  ECF_NOTHROW);
 
   local_define_builtin ("__builtin_nonlocal_goto", ftype,


-- 
Alexandre Oliva, happy hacker  https://FSFLA.org/blogs/lxo/
   Free Software Activist         GNU Toolchain Engineer
        Vim, Vi, Voltei pro Emacs -- GNUlius Caesar

Re: introduce overridable clear_cache emitter

[Jeff Law]

On 12/10/20 3:58 AM, Alexandre Oliva wrote:
> On Dec  3, 2020, Alexandre Oliva <oliva@adacore.com> wrote:
>
>> +    local_define_builtin ("__builtin___clear_cache", ftype,
>> +			  BUILT_IN_CLEAR_CACHE,
>> +			  "__builtin___clear_cache",
>> +			  ECF_NOTHROW);
> Ugh, so that somehow worked for aarch64-linux-gnu-gfortran, but the
> aarch64-elf Ada compiler started issuing calls to an undefined
> __builtin___clear_cache symbol.
>
> The second string actual passed to local_define_builtin binds to formal
> libname, which explains at least the new problem.  I've so far
> restrained my curiosity as to why it wasn't a problem on
> aarch64-linux-gnu-gfortran.  I'm checking it in as obvious, so far
> lightly tested on x86_64-linux-gnu and -x-aarch64-elf.
>
>
> drop __builtin_ from __clear_cache libname
>
> From: Alexandre Oliva <oliva@adacore.com>
>
> I made a cut&pasto in my previous patch for tree.c, causing platforms
> that have CLEAR_INSN_CACHE defined, and none of the internal
> __clear_cache expansion overriders, to issue calls to symbols named
> __builtin___clear_cache rather than __clear_cache, on languages other
> than those in the C family.  Oops.
>
> This patch removes __builtin_ from the string used as the libname for
> __buuiltin___clear_cache.
>
>
> for  gcc/ChangeLog
>
> 	* tree.c (build_common_builtin_nodes): Drop __builtin_ from
> 	__clear_cache libname.
OK
jeff