[PATCH] tcg/ppc: Fix building with Clang

[PATCH] tcg/ppc: Fix building with Clang

[Brad Smith]

Fix building with Clang.

At the moment Clang does not define _CALL_SYSV as GCC does. From
clang/lib/Basic/Targets/PPC.cpp in getTargetDefines()..

  // FIXME: The following are not yet generated here by Clang, but are
  //        generated by GCC:
  //
  //   _SOFT_FLOAT_
  //   __RECIP_PRECISION__
  //   __APPLE_ALTIVEC__
  //   __RECIP__
  //   __RECIPF__
  //   __RSQRTE__
  //   __RSQRTEF__
  //   _SOFT_DOUBLE_
  //   __NO_LWSYNC__
  //   __CMODEL_MEDIUM__
  //   __CMODEL_LARGE__
  //   _CALL_SYSV
  //   _CALL_DARWIN

This is from the OpenBSD ports tree where we use it to build
on OpenBSD/powerpc.

Signed-off-by: Brad Smith <brad@comstyle.com>

diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
index 838ccfa42d..d2611832e5 100644
--- a/tcg/ppc/tcg-target.c.inc
+++ b/tcg/ppc/tcg-target.c.inc
@@ -25,6 +25,11 @@
 #include "elf.h"
 #include "../tcg-pool.c.inc"
 
+/* Clang does not define _CALL_* */
+#if defined(__clang__) && defined(__ELF__) && !defined(_CALL_SYSV)
+#define _CALL_SYSV 1
+#endif
+
 #if defined _CALL_DARWIN || defined __APPLE__
 #define TCG_TARGET_CALL_DARWIN
 #endif

Re: [PATCH] tcg/ppc: Fix building with Clang

[Peter Maydell]

On Wed, 21 Apr 2021 at 02:15, Brad Smith <brad@comstyle.com> wrote:
>
> Fix building with Clang.
>
> At the moment Clang does not define _CALL_SYSV as GCC does. From
> clang/lib/Basic/Targets/PPC.cpp in getTargetDefines()..
>
>   // FIXME: The following are not yet generated here by Clang, but are
>   //        generated by GCC:
>   //
>   //   _SOFT_FLOAT_
>   //   __RECIP_PRECISION__
>   //   __APPLE_ALTIVEC__
>   //   __RECIP__
>   //   __RECIPF__
>   //   __RSQRTE__
>   //   __RSQRTEF__
>   //   _SOFT_DOUBLE_
>   //   __NO_LWSYNC__
>   //   __CMODEL_MEDIUM__
>   //   __CMODEL_LARGE__
>   //   _CALL_SYSV
>   //   _CALL_DARWIN
>
> This is from the OpenBSD ports tree where we use it to build
> on OpenBSD/powerpc.
>
> Signed-off-by: Brad Smith <brad@comstyle.com>
>
> diff --git a/tcg/ppc/tcg-target.c.inc b/tcg/ppc/tcg-target.c.inc
> index 838ccfa42d..d2611832e5 100644
> --- a/tcg/ppc/tcg-target.c.inc
> +++ b/tcg/ppc/tcg-target.c.inc
> @@ -25,6 +25,11 @@
>  #include "elf.h"
>  #include "../tcg-pool.c.inc"
>
> +/* Clang does not define _CALL_* */
> +#if defined(__clang__) && defined(__ELF__) && !defined(_CALL_SYSV)
> +#define _CALL_SYSV 1
> +#endif

This is trying to identify the calling convention used by the OS.
That's not purely compiler specific (ie it is not the case that
all ELF output from clang is definitely using the calling convention
that _CALL_SYSV implies), so settign it purely based on "this is clang
producing ELF files" doesn't seem right.

I guess if clang doesn't reliably tell us the calling convention
maybe we should scrap the use of _CALL_SYSV and _CALL_ELF and
use the host OS defines to guess the calling convention ?

thanks
-- PMM

Re: [PATCH] tcg/ppc: Fix building with Clang

[Richard Henderson]

On 4/21/21 2:03 AM, Peter Maydell wrote:
>> +/* Clang does not define _CALL_* */
>> +#if defined(__clang__) && defined(__ELF__) && !defined(_CALL_SYSV)
>> +#define _CALL_SYSV 1
>> +#endif
> 
> This is trying to identify the calling convention used by the OS.
> That's not purely compiler specific (ie it is not the case that
> all ELF output from clang is definitely using the calling convention
> that _CALL_SYSV implies), so settign it purely based on "this is clang
> producing ELF files" doesn't seem right.

We can get pretty close though.  There are three ppc32 calling conventions: 
AIX, DARWIN, SYSV.  The _CALL_ELF symbol is a 64-bit thing, and AIX itself 
doesn't use ELF.

> I guess if clang doesn't reliably tell us the calling convention
> maybe we should scrap the use of _CALL_SYSV and _CALL_ELF and
> use the host OS defines to guess the calling convention ?

No, I'd rely on _CALL_* first, and only fall back to something else if they're 
not present.

I'm thinking something like

#if !defined(_CALL_SYSV) && \
     !defined(_CALL_DARWIN) && \
     !defined(_CALL_AIX) && \
     !defined(_CALL_ELF)
# if defined(__APPLE__)
#  define _CALL_DARWIN
# elif defined(__ELF__) && TCG_TARGET_REG_BITS == 32
#  define _CALL_SYSV
# else
#  error "Unknown ABI"
# endif
#endif


r~

Re: [PATCH] tcg/ppc: Fix building with Clang

[Peter Maydell]

On Thu, 22 Apr 2021 at 06:18, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 4/21/21 2:03 AM, Peter Maydell wrote:
> >> +/* Clang does not define _CALL_* */
> >> +#if defined(__clang__) && defined(__ELF__) && !defined(_CALL_SYSV)
> >> +#define _CALL_SYSV 1
> >> +#endif
> >
> > This is trying to identify the calling convention used by the OS.
> > That's not purely compiler specific (ie it is not the case that
> > all ELF output from clang is definitely using the calling convention
> > that _CALL_SYSV implies), so settign it purely based on "this is clang
> > producing ELF files" doesn't seem right.
>
> We can get pretty close though.  There are three ppc32 calling conventions:
> AIX, DARWIN, SYSV.  The _CALL_ELF symbol is a 64-bit thing, and AIX itself
> doesn't use ELF.
>
> > I guess if clang doesn't reliably tell us the calling convention
> > maybe we should scrap the use of _CALL_SYSV and _CALL_ELF and
> > use the host OS defines to guess the calling convention ?
>
> No, I'd rely on _CALL_* first, and only fall back to something else if they're
> not present.
>
> I'm thinking something like
>
> #if !defined(_CALL_SYSV) && \
>      !defined(_CALL_DARWIN) && \
>      !defined(_CALL_AIX) && \
>      !defined(_CALL_ELF)
> # if defined(__APPLE__)
> #  define _CALL_DARWIN
> # elif defined(__ELF__) && TCG_TARGET_REG_BITS == 32
> #  define _CALL_SYSV
> # else
> #  error "Unknown ABI"
> # endif
> #endif

Doesn't this also need some case that handles "64bit ppc clang which doesn't
define _CALL_anything" ?

thanks
-- PMM

Re: [PATCH] tcg/ppc: Fix building with Clang

[Richard Henderson]

On 4/22/21 2:20 AM, Peter Maydell wrote:
> On Thu, 22 Apr 2021 at 06:18, Richard Henderson
>> I'm thinking something like
>>
>> #if !defined(_CALL_SYSV) && \
>>       !defined(_CALL_DARWIN) && \
>>       !defined(_CALL_AIX) && \
>>       !defined(_CALL_ELF)
>> # if defined(__APPLE__)
>> #  define _CALL_DARWIN
>> # elif defined(__ELF__) && TCG_TARGET_REG_BITS == 32
>> #  define _CALL_SYSV
>> # else
>> #  error "Unknown ABI"
>> # endif
>> #endif
> 
> Doesn't this also need some case that handles "64bit ppc clang which doesn't
> define _CALL_anything" ?

Clang does define _CALL_ELF for ppc64:

   // ABI options.
   if (ABI == "elfv1")
     Builder.defineMacro("_CALL_ELF", "1");
   if (ABI == "elfv2")
     Builder.defineMacro("_CALL_ELF", "2");


r~

Re: [PATCH] tcg/ppc: Fix building with Clang

[Brad Smith]

On 4/22/2021 11:39 AM, Richard Henderson wrote:
> On 4/22/21 2:20 AM, Peter Maydell wrote:
>> On Thu, 22 Apr 2021 at 06:18, Richard Henderson
>>> I'm thinking something like
>>>
>>> #if !defined(_CALL_SYSV) && \
>>>       !defined(_CALL_DARWIN) && \
>>>       !defined(_CALL_AIX) && \
>>>       !defined(_CALL_ELF)
>>> # if defined(__APPLE__)
>>> #  define _CALL_DARWIN
>>> # elif defined(__ELF__) && TCG_TARGET_REG_BITS == 32
>>> #  define _CALL_SYSV
>>> # else
>>> #  error "Unknown ABI"
>>> # endif
>>> #endif
>>
>> Doesn't this also need some case that handles "64bit ppc clang which 
>> doesn't
>> define _CALL_anything" ?
>
> Clang does define _CALL_ELF for ppc64:
>
>  // ABI options.
>  if (ABI == "elfv1")
>    Builder.defineMacro("_CALL_ELF", "1");
>  if (ABI == "elfv2")
>    Builder.defineMacro("_CALL_ELF", "2");
>

Able to spin up a patch that you think is appropriate?

Re: [PATCH] tcg/ppc: Fix building with Clang

[Brad Smith]

On 5/2/2021 12:02 AM, Brad Smith wrote:

> On 4/22/2021 11:39 AM, Richard Henderson wrote:
>> On 4/22/21 2:20 AM, Peter Maydell wrote:
>>> On Thu, 22 Apr 2021 at 06:18, Richard Henderson
>>>> I'm thinking something like
>>>>
>>>> #if !defined(_CALL_SYSV) && \
>>>>       !defined(_CALL_DARWIN) && \
>>>>       !defined(_CALL_AIX) && \
>>>>       !defined(_CALL_ELF)
>>>> # if defined(__APPLE__)
>>>> #  define _CALL_DARWIN
>>>> # elif defined(__ELF__) && TCG_TARGET_REG_BITS == 32
>>>> #  define _CALL_SYSV
>>>> # else
>>>> #  error "Unknown ABI"
>>>> # endif
>>>> #endif
>>>
>>> Doesn't this also need some case that handles "64bit ppc clang which 
>>> doesn't
>>> define _CALL_anything" ?
>>
>> Clang does define _CALL_ELF for ppc64:
>>
>>  // ABI options.
>>  if (ABI == "elfv1")
>>    Builder.defineMacro("_CALL_ELF", "1");
>>  if (ABI == "elfv2")
>>    Builder.defineMacro("_CALL_ELF", "2");
>>
>
> Able to spin up a patch that you think is appropriate?

ping?