[Sparc] Define the same macros for -mcpu=v9 as GCC on Linux and the BSDs

[glaubitz]

Summary

When targeting SPARC V8+ on Linux and the BSDs, GCC defines the macro
sparc_v9 only while clang also defines additional macros such as
sparcv9. When targeting SPARC V9, GCC defines sparc_v9__ on Linux
and the BSDs while other macros such as sparcv9 are defined on FreeBSD,
NetBSD and Solaris. On OpenBSD, GCC defines sparcv9__ and not __sparcv9.

In order to avoid compatibility problems, make sure clang behaves as
GCC and defines the same macros depending on the target platform and
SPARC CPU baseline. Update the tests accordingly.

Fixes PR49562.

[brad]

I do not immediately see why the other tests are failing, but at a bare minimum the following test from clang/test/Preprocessor/predefined-arch-macros.c will have to be updated..

// RUN: %clang -mcpu=v9 -E -dM %s -o - 2>&1 \
// RUN:     -target sparc-unknown-linux \
// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_SPARC-V9
// CHECK_SPARC-V9-NOT: #define __sparcv8 1
// CHECK_SPARC-V9-NOT: #define __sparcv8__ 1
// CHECK_SPARC-V9: #define __sparc_v9__ 1
// CHECK_SPARC-V9: #define __sparcv9 1
// CHECK_SPARC-V9: #define __sparcv9__ 1

[jrtc27]

clang/lib/Basic/Targets/Sparc.cpp

160–164

This is wrong, they should never be defined on any OS for 32-bit SPARC; see https://github.com/gcc-mirror/gcc/blob/master/gcc/config/sparc/netbsd-elf.h#L22 for example (FreeBSD only ever had a 64-bit port so isn't interesting to look at). That is, the contents of this if should just be deleted, leaving just __sparc_v9__.

247–259

This doesn't need changing, we can define more things than GCC to keep it simple.

[glaubitz]

clang/lib/Basic/Targets/Sparc.cpp

160–164

Agreed although I haven't verified whether `sparc_v9`` is defined for NetBSD with `-m32 -mcpu=v9``.

247–259

Well, my original intent was to match GCC to make sure we're 100% compatible and I would like to keep it that way.

[glaubitz]

In D98574#2624558, @brad wrote:

I do not immediately see why the other tests are failing, but at a bare minimum the following test from clang/test/Preprocessor/predefined-arch-macros.c will have to be updated..

// RUN: %clang -mcpu=v9 -E -dM %s -o - 2>&1 \
// RUN:     -target sparc-unknown-linux \
// RUN:   | FileCheck -match-full-lines %s -check-prefix=CHECK_SPARC-V9
// CHECK_SPARC-V9-NOT: #define __sparcv8 1
// CHECK_SPARC-V9-NOT: #define __sparcv8__ 1
// CHECK_SPARC-V9: #define __sparc_v9__ 1
// CHECK_SPARC-V9: #define __sparcv9 1
// CHECK_SPARC-V9: #define __sparcv9__ 1

Thanks, I agree these need to be adjusted.

[glaubitz]

For reference, here is what GCC defines on Linux with regards to SPARC:

glaubitz@gcc202:~$ echo | gcc -E -dM -mcpu=v9 -m32 - |grep arch
glaubitz@gcc202:~$ echo | gcc -E -dM -mcpu=v9 -m32 - |grep LP
glaubitz@gcc202:~$ echo | gcc -E -dM -mcpu=v9 -m32 - |grep v9
#define __sparc_v9__ 1
glaubitz@gcc202:~$ echo | gcc -E -dM -mcpu=v9 -m64 - |grep arch
#define __arch64__ 1
glaubitz@gcc202:~$ echo | gcc -E -dM -mcpu=v9 -m64 - |grep LP
#define __LP64__ 1
#define _LP64 1
glaubitz@gcc202:~$ echo | gcc -E -dM -mcpu=v9 -m64 - |grep v9
#define __sparc_v9__ 1
glaubitz@gcc202:~$

And here is what it defines on OpenBSD:

openbsd# echo | gcc -E -dM -mcpu=v9 -m64 - |grep arch 
#define __arch64__ 1
openbsd# echo | gcc -E -dM -mcpu=v9 -m64 - |grep LP   
#define __LP64__ 1
#define _LP64 1
openbsd# echo | gcc -E -dM -mcpu=v9 -m64 - |grep sparc
#define sparc 1
#define __sparc__ 1
#define __sparc 1
#define __sparc64__ 1
#define __sparcv9__ 1
#define __sparc_v9__ 1
openbsd#

-m32 is not supported on OpenBSD at all:

openbsd# echo | gcc -E -dM -m32 -            
:0: error: -m32 is not supported by this configuration
openbsd#

For NetBSD, we have:

$ echo | gcc -E -dM -mcpu=v9 -m32 - |grep sparc
#define sparc 1
#define __sparc__ 1
#define __sparc 1
#define __sparc_v9__ 1
$ echo | gcc -E -dM -mcpu=v9 -m64 - |grep sparc
#define sparc 1
#define __sparc__ 1
#define __sparcv9 1
#define __sparc 1
#define __sparc64__ 1
#define __sparc_v9__ 1
$ echo | gcc -E -dM -mcpu=v9 -m32 - |grep LP
$ echo | gcc -E -dM -mcpu=v9 -m64 - | grep LP
#define __LP64__ 1
#define _LP64 1
$ echo | gcc -E -dM -mcpu=v9 -m32 - | grep arch
$ echo | gcc -E -dM -mcpu=v9 -m64 - | grep arch
#define __arch64__ 1

And with the native 32-bit compiler on NetBSD, we get:

$ echo | gcc -E -dM -mcpu=v9  - | grep sparc
#define sparc 1
#define __sparc__ 1
#define __sparc 1
#define __sparc_v9__ 1
$ echo | gcc -E -dM -mcpu=v9 - | grep LP
$ echo | gcc -E -dM -mcpu=v9 - | grep arch

So, I think clang behaves correctly with the changes in this diff - although it does not account for the difference between OpenBSD and NetBSD.

[brad]

In D98574#2641697, @glaubitz wrote:

-m32 is not supported on OpenBSD at all:

openbsd# echo | gcc -E -dM -m32 -            
:0: error: -m32 is not supported by this configuration
openbsd#

Yes. OpenBSD has no 32-bit support for sparc64 (nor does FreeBSD).

[glaubitz]

I have updated the patch now with the result that clang should behave as GCC now on Linux, NetBSD and OpenBSD.

[glaubitz]

Updated as I previously forgot to account for FreeBSD as well.

[ro]

clang/lib/Basic/Targets/Sparc.cpp

247–259

I agree with Jessica here: you're creating a complicated maze for no real gain. Besides, have you checked what gcc on the BSDs really does? They often neglect to get their changes upstream and what's in the gcc repo doesn't necessarily represent what they actually use.

[glaubitz]

clang/lib/Basic/Targets/Sparc.cpp

247–259

Yes, I have verified that GCC behaves the exact same way as this change and I don't see any reason not to mimic the exact same behavior in clang for maximum compatibility.

[glaubitz]

clang/lib/Basic/Targets/Sparc.cpp

247–259

FWIW, I meant GCC on the various BSDs. I do not think it's a wise idea to have clang deviate from what GCC does as only this way we can guarantee that everything that compiles with GCC will compile with clang.

[brad]

clang/lib/Basic/Targets/Sparc.cpp

247–259

Besides, have you checked what gcc on the BSDs really does? They often neglect to get their changes upstream and what's in the gcc repo doesn't necessarily represent what they actually use.

What is upstream is what we do. There are no local patches that change behavior in this particular area.

[glaubitz]

Gentle ping.

[joerg]

The NetBSD part looks ok, but also lacks proper testing. I'm not sure anyone but Linux cares at all otherwise as they lack 32bit SPARC support.

[glaubitz]

In D98574#2654686, @joerg wrote:

The NetBSD part looks ok, but also lacks proper testing. I'm not sure anyone but Linux cares at all otherwise as they lack 32bit SPARC support.

Well, there were no tests for NetBSD before, so I didn't change anything in this regard. I only changed clang to behave as gcc on SPARC and I
think that's the reasonable thing to do to avoid any compatibility issues when building with clang instead of gcc on any SPARC target.

[glaubitz]

Ping.

[hvdijk]

clang/lib/Basic/Targets/Sparc.cpp

247–259

(Copying here what I had already replied privately a while back) It worries me that this switch statement only handles known operating systems (Linux, FreeBSD, NetBSD, OpenBSD, Solaris) when we also have code to allow SparcV9TargetInfo to be created without an operating system in clang/lib/Basic/Targets.cpp. Either there should be a default case that is properly handled, or if that actually cannot happen, there should be an assert that it doesn't happen.

I agree with the earlier comments that there should be nothing wrong with defining more macros than GCC, if the macros make sense. For the SparcV9TargetInfo class, my impression is that the macros make sense. For the SparcV8TargetInfo class with a V9 CPU, reading the discussion in D86621, if Oracle say that __sparcv9 is only for 64-bit mode, GCC also only defines it for 64-bit mode, glibc assumes that __sparcv9 implies 64-bit mode, etc. then SparcV8TargetInfo should not be defining __sparcv9. Your changes to SparcV8TargetInfo should be enough to fix bug 49562, right? If so, would it be okay to update this diff with just that?