This is an archive of the discontinued LLVM Phabricator instance.

Differential D103587

[AIX] Define __TOS_AIX__ predefined macro
ClosedPublic

Authored by Jake-Egan on Jun 2 2021, 8:48 PM.

Details

Reviewers
hubert.reinterpretcast
cebowleratibm
Commits
rG045872f42203: [AIX] Define __TOS_AIX__ predefined macro
Summary


Transfer the predefined macro, __TOS_AIX__, from the AIX XL C/C++ compilers.

__TOS_AIX__ indicates that the target operating system is AIX.

Diff Detail

Unit TestsFailed

TimeTest
34,830 msx64 debian > libFuzzer.libFuzzer::entropic-scale-per-exec-time.test

Event Timeline

Jake-Egan created this revision.Jun 2 2021, 8:48 PM
Herald added subscribers: jfb, kbarton, nemanjai. · View Herald TranscriptJun 2 2021, 8:48 PM
Jake-Egan requested review of this revision.Jun 2 2021, 8:48 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 2 2021, 8:48 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
Jake-Egan retitled this revision from Add AIX predefined macros to [AIX] Transfer predefined macros.Jun 2 2021, 8:53 PM
Jake-Egan edited the summary of this revision. (Show Details)
Jake-Egan added reviewers: hubert.reinterpretcast, cebowleratibm.
Jake-Egan edited the summary of this revision. (Show Details)
Harbormaster completed remote builds in B107376: Diff 349445.Jun 2 2021, 9:29 PM
cebowleratibm requested changes to this revision.Jun 3 2021, 7:09 AM

Please rework the test update into init-ppc.c

clang/lib/Basic/Targets/PPC.cpp
95 ↗(On Diff #349445)

The XL compilers define this macro for Power but GCC does not. The macro is redundant and I recommend we not carry it forward to Linux. There's probably a case to define it on AIX where the prevalent XL compilers have always defined it.

clang/test/Preprocessor/init-aix.c
1 ↗(On Diff #349445)

There are some AIX macro tests already in init-ppc.c. Note the target triple should be specified or this test will break when run on a non AIX host.

This revision now requires changes to proceed.Jun 3 2021, 7:09 AM
Jake-Egan updated this revision to Diff 349557.EditedJun 3 2021, 8:33 AM
Jake-Egan edited the summary of this revision. (Show Details)

Removed

__THW_PPC__

and moved tests to init-ppc.c

Harbormaster completed remote builds in B107458: Diff 349557.Jun 3 2021, 8:33 AM
Jake-Egan updated this revision to Diff 349564.EditedJun 3 2021, 8:48 AM
Jake-Egan marked an inline comment as done.

Fix diff

Harbormaster completed remote builds in B107463: Diff 349564.Jun 3 2021, 9:22 AM
cebowleratibm requested changes to this revision.Jun 3 2021, 10:22 AM
cebowleratibm added inline comments.
clang/lib/Basic/Targets/OSTargets.h
680

Should set these macros in C11 and higher levels.

clang/test/Preprocessor/init-ppc.c
544

I think it's sufficient to test the STDC_NO_ATOMICS and STDC_NO_THREADS only as you've added below where the -std is explicitly controlled. I prefer to avoid expecting these macros for the -std default, which may change.

727

suggest adding the negative test for pre C11 modes.

This revision now requires changes to proceed.Jun 3 2021, 10:22 AM
Jake-Egan updated this revision to Diff 349638.Jun 3 2021, 12:10 PM

Set macros for C11 and higher. Add negative test for pre C11.

Jake-Egan edited the summary of this revision. (Show Details)Jun 3 2021, 12:41 PM
Harbormaster completed remote builds in B107516: Diff 349638.Jun 3 2021, 1:11 PM
cebowleratibm added inline comments.Jun 3 2021, 2:29 PM
clang/lib/Basic/Targets/OSTargets.h
680

I think LangStandard::isC11() does what you want. You wouldn't want to update this code every time a new C standard comes out.

clang/test/Preprocessor/init-ppc.c
742

I didn't mean to suggest adding all of them. One negative test on c99 would suffice.

Jake-Egan updated this revision to Diff 349854.Jun 4 2021, 6:34 AM
Jake-Egan marked 3 inline comments as done.
This comment was removed by Jake-Egan.
Jake-Egan updated this revision to Diff 349855.Jun 4 2021, 6:42 AM

Use LangStandard::isC11() and one C99 test.

Harbormaster completed remote builds in B107671: Diff 349855.Jun 4 2021, 7:19 AM
cebowleratibm requested changes to this revision.Jun 4 2021, 7:26 AM

I think it makes sense to split the STDC macro changes and TOS_AIX__ into different patches.

This revision now requires changes to proceed.Jun 4 2021, 7:26 AM
Jake-Egan retitled this revision from [AIX] Transfer predefined macros to [AIX] Transfer __TOS_AIX__ predefined macro.Jun 4 2021, 10:22 AM
Jake-Egan edited the summary of this revision. (Show Details)
Jake-Egan edited the summary of this revision. (Show Details)
Jake-Egan updated this revision to Diff 349913.EditedJun 4 2021, 10:29 AM


Use this patch to target only __TOS_AIX__.

Harbormaster completed remote builds in B107703: Diff 349913.Jun 4 2021, 11:17 AM
Jake-Egan retitled this revision from [AIX] Transfer __TOS_AIX__ predefined macro to [AIX] Define __TOS_AIX__ predefined macro.Jun 4 2021, 11:56 AM
Jake-Egan mentioned this in D103707: [AIX] Define __STDC_NO_ATOMICS__ and __STDC_NO_THREADS__ predefined macros.Jun 4 2021, 12:03 PM
cebowleratibm accepted this revision.Jul 6 2021, 7:03 AM

LGTM

This revision is now accepted and ready to land.Jul 6 2021, 7:03 AM
Closed by commit rG045872f42203: [AIX] Define __TOS_AIX__ predefined macro (authored by Jake-Egan, committed by stevewan). · Explain WhyJul 6 2021, 11:16 AM
This revision was automatically updated to reflect the committed changes.
stevewan added a commit: rG045872f42203: [AIX] Define __TOS_AIX__ predefined macro.

Revision Contents

PathSize
clang/
lib/
Basic/
Targets/
6 lines
test/
Preprocessor/
20 lines

Diff 349855

clang/lib/Basic/Targets/OSTargets.h

Show First 20 Lines • Show All 669 Lines • ▼ Show 20 Lines
protected: protected:
void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple, void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple,
MacroBuilder &Builder) const override { MacroBuilder &Builder) const override {
DefineStd(Builder, "unix", Opts); DefineStd(Builder, "unix", Opts);
Builder.defineMacro("_IBMR2"); Builder.defineMacro("_IBMR2");
Builder.defineMacro("_POWER"); Builder.defineMacro("_POWER");
Builder.defineMacro("_AIX"); Builder.defineMacro("_AIX");
Builder.defineMacro("__TOS_AIX__");
if (LangStandard::getLangStandardForKind(Opts.LangStd).isC11()) {
cebowleratibmUnsubmitted
Done
ReplyInline Actions

Should set these macros in C11 and higher levels.

cebowleratibm: Should set these macros in C11 and higher levels.
cebowleratibmUnsubmitted
Not Done
ReplyInline Actions

I think LangStandard::isC11() does what you want. You wouldn't want to update this code every time a new C standard comes out.

cebowleratibm: I think LangStandard::isC11() does what you want. You wouldn't want to update this code every…
Builder.defineMacro("__STDC_NO_ATOMICS__");
Builder.defineMacro("__STDC_NO_THREADS__");
}
if (Opts.EnableAIXExtendedAltivecABI) if (Opts.EnableAIXExtendedAltivecABI)
Builder.defineMacro("__EXTABI__"); Builder.defineMacro("__EXTABI__");
unsigned Major, Minor, Micro; unsigned Major, Minor, Micro;
Triple.getOSVersion(Major, Minor, Micro); Triple.getOSVersion(Major, Minor, Micro);
// Define AIX OS-Version Macros. // Define AIX OS-Version Macros.
▲ Show 20 Lines • Show All 267 Lines • Show Last 20 Lines

clang/test/Preprocessor/init-ppc.c

Show First 20 Lines • Show All 535 Lines • ▼ Show 20 Lines
// PPC-AIX:#define __SIZEOF_PTRDIFF_T__ 4 // PPC-AIX:#define __SIZEOF_PTRDIFF_T__ 4
// PPC-AIX:#define __SIZEOF_SHORT__ 2 // PPC-AIX:#define __SIZEOF_SHORT__ 2
// PPC-AIX:#define __SIZEOF_SIZE_T__ 4 // PPC-AIX:#define __SIZEOF_SIZE_T__ 4
// PPC-AIX:#define __SIZEOF_WCHAR_T__ 2 // PPC-AIX:#define __SIZEOF_WCHAR_T__ 2
// PPC-AIX:#define __SIZEOF_WINT_T__ 4 // PPC-AIX:#define __SIZEOF_WINT_T__ 4
// PPC-AIX:#define __SIZE_MAX__ 4294967295UL // PPC-AIX:#define __SIZE_MAX__ 4294967295UL
// PPC-AIX:#define __SIZE_TYPE__ long unsigned int // PPC-AIX:#define __SIZE_TYPE__ long unsigned int
// PPC-AIX:#define __SIZE_WIDTH__ 32 // PPC-AIX:#define __SIZE_WIDTH__ 32
// PPC-AIX:#define __TOS_AIX__ 1
cebowleratibmUnsubmitted
Done
ReplyInline Actions

I think it's sufficient to test the STDC_NO_ATOMICS and STDC_NO_THREADS only as you've added below where the -std is explicitly controlled. I prefer to avoid expecting these macros for the -std default, which may change.

cebowleratibm: I think it's sufficient to test the __STDC_NO_ATOMICS__ and __STDC_NO_THREADS__ only as you've…
// PPC-AIX:#define __UINT16_C_SUFFIX__ // PPC-AIX:#define __UINT16_C_SUFFIX__
// PPC-AIX:#define __UINT16_MAX__ 65535 // PPC-AIX:#define __UINT16_MAX__ 65535
// PPC-AIX:#define __UINT16_TYPE__ unsigned short // PPC-AIX:#define __UINT16_TYPE__ unsigned short
// PPC-AIX:#define __UINT32_C_SUFFIX__ U // PPC-AIX:#define __UINT32_C_SUFFIX__ U
// PPC-AIX:#define __UINT32_MAX__ 4294967295U // PPC-AIX:#define __UINT32_MAX__ 4294967295U
// PPC-AIX:#define __UINT32_TYPE__ unsigned int // PPC-AIX:#define __UINT32_TYPE__ unsigned int
// PPC-AIX:#define __UINT64_C_SUFFIX__ ULL // PPC-AIX:#define __UINT64_C_SUFFIX__ ULL
// PPC-AIX:#define __UINT64_MAX__ 18446744073709551615ULL // PPC-AIX:#define __UINT64_MAX__ 18446744073709551615ULL
▲ Show 20 Lines • Show All 166 Lines • ▼ Show 20 Lines
// PPC-AIX-NOWCHAR-NOT:#define _WCHAR_T 1 // PPC-AIX-NOWCHAR-NOT:#define _WCHAR_T 1
// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char -pthread < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-THREADSAFE %s // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char -pthread < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-THREADSAFE %s
// PPC-AIX-THREADSAFE:#define _THREAD_SAFE 1 // PPC-AIX-THREADSAFE:#define _THREAD_SAFE 1
// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-NOTHREADSAFE %s // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-NOTHREADSAFE %s
// PPC-AIX-NOTHREADSAFE-NOT:#define _THREAD_SAFE 1 // PPC-AIX-NOTHREADSAFE-NOT:#define _THREAD_SAFE 1
// RUN: %clang_cc1 -x c -std=c11 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-STDC %s
cebowleratibmUnsubmitted
Done
ReplyInline Actions

suggest adding the negative test for pre C11 modes.

cebowleratibm: suggest adding the negative test for pre C11 modes.
// RUN: %clang_cc1 -x c -std=c1x -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-STDC %s
// RUN: %clang_cc1 -x c -std=iso9899:2011 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.10.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-STDC %s
// RUN: %clang_cc1 -x c -std=gnu11 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-STDC %s
// RUN: %clang_cc1 -x c -std=c17 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-STDC %s
// RUN: %clang_cc1 -x c -std=iso9899:2017 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-STDC %s
// RUN: %clang_cc1 -x c -std=c18 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-STDC %s
// RUN: %clang_cc1 -x c -std=iso9899:2018 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-STDC %s
// RUN: %clang_cc1 -x c -std=gnu17 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-STDC %s
// RUN: %clang_cc1 -x c -std=gnu18 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-STDC %s
// RUN: %clang_cc1 -x c -std=c2x -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-STDC %s
// RUN: %clang_cc1 -x c -std=gnu2x -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-STDC %s
// PPC-AIX-STDC:#define __STDC_NO_ATOMICS__ 1
// PPC-AIX-STDC:#define __STDC_NO_THREADS__ 1
// RUN: %clang_cc1 -x c -std=c99 -E -dM -ffreestanding -triple=powerpc-ibm-aix7.1.0.0 -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-AIX-STDC-N %s
cebowleratibmUnsubmitted
Not Done
ReplyInline Actions

I didn't mean to suggest adding all of them. One negative test on c99 would suffice.

cebowleratibm: I didn't mean to suggest adding all of them. One negative test on c99 would suffice.
// PPC-AIX-STDC-N-NOT:#define __STDC_NO_ATOMICS__ 1
// PPC-AIX-STDC-N-NOT:#define __STDC_NO_THREADS__ 1
// RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-unknown-linux-gnu -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-LINUX %s // RUN: %clang_cc1 -E -dM -ffreestanding -triple=powerpc-unknown-linux-gnu -fno-signed-char < /dev/null | FileCheck -match-full-lines -check-prefix PPC-LINUX %s
// //
// PPC-LINUX:#define _ARCH_PPC 1 // PPC-LINUX:#define _ARCH_PPC 1
// PPC-LINUX:#define _BIG_ENDIAN 1 // PPC-LINUX:#define _BIG_ENDIAN 1
// PPC-LINUX-NOT:#define _LP64 // PPC-LINUX-NOT:#define _LP64
// PPC-LINUX:#define __BIGGEST_ALIGNMENT__ 16 // PPC-LINUX:#define __BIGGEST_ALIGNMENT__ 16
// PPC-LINUX:#define __BIG_ENDIAN__ 1 // PPC-LINUX:#define __BIG_ENDIAN__ 1
// PPC-LINUX:#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__ // PPC-LINUX:#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__
▲ Show 20 Lines • Show All 399 Lines • Show Last 20 Lines