This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: RFA: Remove alias usage from libgcc/sync.c
- From: Richard Sandiford <rdsandiford at googlemail dot com>
- To: Richard Biener <richard dot guenther at gmail dot com>
- Cc: Jakub Jelinek <jakub at redhat dot com>, GCC Patches <gcc-patches at gcc dot gnu dot org>, Jan Hubicka <hubicka at ucw dot cz>
- Date: Tue, 15 Oct 2013 08:59:22 +0100
- Subject: Re: RFA: Remove alias usage from libgcc/sync.c
- Authentication-results: sourceware.org; auth=none
- References: <87ob6wp8oh dot fsf at talisman dot default> <CAFiYyc1cNcSgwXP4=OySk8d_3Wwx5uF4x=c3QEfjS_t3zsUZwg at mail dot gmail dot com> <20131011082136 dot GO30970 at tucnak dot zalov dot cz> <87bo2wta5l dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <CAFiYyc3S3n0UK6WCdv4eWuLUA59HNDUbEv3bnGBM8MN9QPCECg at mail dot gmail dot com> <87a9igrsli dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <CAFiYyc3e=dyLppc9LPSj7rmxz0KeQ=s=Wqx8hK1YC8Y14vmGbg at mail dot gmail dot com> <877gdkos42 dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <CAFiYyc1UPOwfaMfY9wJnf4YW1zrBmZeus4OEQZYcfXOx+6D7eA at mail dot gmail dot com>
Richard Biener <richard.guenther@gmail.com> writes:
>>> Honza? For tailr this boils down to symtab_semantically_equivalent_p ().
>>> I suppose we don't want to change that but eventually special case
>>> builtins in tailr, thus
>>>
>>> Index: gcc/tree-tailcall.c
>>> ===================================================================
>>> --- gcc/tree-tailcall.c (revision 203409)
>>> +++ gcc/tree-tailcall.c (working copy)
>>> @@ -446,7 +446,9 @@ find_tail_calls (basic_block bb, struct
>>> /* We found the call, check whether it is suitable. */
>>> tail_recursion = false;
>>> func = gimple_call_fndecl (call);
>>> - if (func && recursive_call_p (current_function_decl, func))
>>> + if (func
>>> + && ! DECL_BUILT_IN (func)
>>> + && recursive_call_p (current_function_decl, func))
>>> {
>>> tree arg;
>>>
>>> which makes -O2 not turn it into an infinite loop (possibly also applies
>>> to the original code with the alias declaration?)
>>
>> If that's OK then I'm certainly happy with it :-)
>
> I'm happy with the tailcall change (if you can do the testing ... ;))
Thanks.
>> FWIW, the alias case
>> was first optimised from __sync_synchronize->sync_synchronize, before it
>> got converted into a tail call. That happened very early in the pipeline
>> and I suppose would stop the built-in expansion from kicking in,
>> even with tail calls disabled. But if we say that:
>>
>> foo () { foo (); }
>>
>> is a supported way of defining out-of-line versions of built-in foo,
>> then that's much more convenient than the aliases anyway.
>
> Btw, if it is supported then we should add a testcase that makes sure
> we don't regress for this. (I wouldn't say we should document this
> "feature" just in case we want to decide it's not the way we want it
> in the future).
OK, I adjusted the x86 test I posted on gcc@ (and fixed the \; problem
Jakub pointed out).
Tested on x86_64-linux-gnu and mips64-linux-gnu. OK to install?
Thanks,
Richard
gcc/
2013-10-15 Richard Biener <rguenther@suse.de>
* tree-tailcall.c (find_tail_calls): Don't use tail-call recursion
for built-in functions.
gcc/testsuite/
* gcc.dg/torture/builtin-self.c: New file.
libgcc/
* sync.c: Remove static aliases and define each function directly
under its real name.
Index: gcc/tree-tailcall.c
===================================================================
--- gcc/tree-tailcall.c 2013-10-15 08:52:07.358853220 +0100
+++ gcc/tree-tailcall.c 2013-10-15 08:53:06.980419473 +0100
@@ -446,7 +446,9 @@ find_tail_calls (basic_block bb, struct
/* We found the call, check whether it is suitable. */
tail_recursion = false;
func = gimple_call_fndecl (call);
- if (func && recursive_call_p (current_function_decl, func))
+ if (func
+ && !DECL_BUILT_IN (func)
+ && recursive_call_p (current_function_decl, func))
{
tree arg;
Index: gcc/testsuite/gcc.dg/torture/builtin-self.c
===================================================================
--- /dev/null 2013-10-13 08:29:45.608935301 +0100
+++ gcc/testsuite/gcc.dg/torture/builtin-self.c 2013-10-15 08:58:00.064045777 +0100
@@ -0,0 +1,10 @@
+/* { dg-do compile { target i?86-*-* x86_64-*-* } } */
+/* Check that we can use this idiom to define out-of-line copies of built-in
+ functions. This is used by libgcc/sync.c, for example. */
+void __sync_synchronize (void)
+{
+ __sync_synchronize ();
+}
+/* { dg-final { scan-assembler "__sync_synchronize" } } */
+/* { dg-final { scan-assembler "\t(lock|mfence)" } } */
+/* { dg-final { scan-assembler-not "\tcall" } } */
Index: libgcc/sync.c
===================================================================
--- libgcc/sync.c 2013-10-15 08:52:07.358853220 +0100
+++ libgcc/sync.c 2013-10-15 08:53:06.981419482 +0100
@@ -72,22 +72,22 @@ Software Foundation; either version 3, o
TYPE is a type that has UNITS bytes. */
#define DEFINE_V_PV(NAME, UNITS, TYPE) \
- static TYPE \
- NAME##_##UNITS (TYPE *ptr, TYPE value) \
+ TYPE \
+ __##NAME##_##UNITS (TYPE *ptr, TYPE value) \
{ \
return __##NAME (ptr, value); \
}
-#define DEFINE_V_PVV(NAME, UNITS, TYPE) \
- static TYPE \
- NAME##_##UNITS (TYPE *ptr, TYPE value1, TYPE value2) \
+#define DEFINE_V_PVV(NAME, UNITS, TYPE) \
+ TYPE \
+ __##NAME##_##UNITS (TYPE *ptr, TYPE value1, TYPE value2) \
{ \
return __##NAME (ptr, value1, value2); \
}
#define DEFINE_BOOL_PVV(NAME, UNITS, TYPE) \
- static _Bool \
- NAME##_##UNITS (TYPE *ptr, TYPE value1, TYPE value2) \
+ _Bool \
+ __##NAME##_##UNITS (TYPE *ptr, TYPE value1, TYPE value2) \
{ \
return __##NAME (ptr, value1, value2); \
}
@@ -118,9 +118,7 @@ #define local_sync_lock_test_and_set DEF
#define DEFINE1(NAME, UNITS, TYPE) \
static int unused[sizeof (TYPE) == UNITS ? 1 : -1] \
__attribute__((unused)); \
- local_##NAME (NAME, UNITS, TYPE); \
- typeof (NAME##_##UNITS) __##NAME##_##UNITS \
- __attribute__((alias (#NAME "_" #UNITS)));
+ local_##NAME (NAME, UNITS, TYPE);
/* As above, but performing macro expansion on the arguments. */
#define DEFINE(NAME, UNITS, TYPE) DEFINE1 (NAME, UNITS, TYPE)
@@ -167,13 +165,11 @@ DEFINE (FN, 8, UOItype)
#if defined Lsync_synchronize
-static void
-sync_synchronize (void)
+void
+__sync_synchronize (void)
{
__sync_synchronize ();
}
-typeof (sync_synchronize) __sync_synchronize \
- __attribute__((alias ("sync_synchronize")));
#endif