Re: introduce overridable clear_cache emitter
On 12/10/20 3:58 AM, Alexandre Oliva wrote:
> On Dec 3, 2020, Alexandre Oliva 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
>
> I made a cut 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
Re: introduce overridable clear_cache emitter
On Dec 3, 2020, Alexandre Oliva 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
I made a cut 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
On Dec 5, 2020, Jakub Jelinek wrote: > On Sat, Dec 05, 2020 at 06:01:59PM -0300, Alexandre Oliva wrote: >> On Dec 5, 2020, Andreas Schwab 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
On Sat, Dec 05, 2020 at 06:01:59PM -0300, Alexandre Oliva wrote: > On Dec 5, 2020, Andreas Schwab 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
On Dec 5, 2020, Andreas Schwab 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
../../../../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, sch...@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
On 12/3/20 7:08 AM, Alexandre Oliva wrote: > On Dec 3, 2020, Christophe Lyon wrote: > >> This patches causes a lot of regressions in fortran on arm and aarch64, > On Dec 3, 2020, Andreas Schwab 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 > > 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
On Dec 3, 2020, Christophe Lyon wrote:
> This patches causes a lot of regressions in fortran on arm and aarch64,
On Dec 3, 2020, Andreas Schwab 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
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
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, sch...@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
../../../../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, sch...@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
On Dec 3, 2020, Christophe Lyon 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
Hi Alexandre, On Wed, 2 Dec 2020 at 19:23, Jeff Law via Gcc-patches 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. > >
Re: introduce overridable clear_cache emitter
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
Hi Alex, Thanks for your proposal on this! > On 11 Nov 2020, at 03:35, 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. 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
