[Clang][NFC] Add threadsafe macro helper

[daltenty]

This PR does a NFC cleanup to pull the common logic for setting threadsafe macros (i.e. the pthread is present) into a helper function.

This will enable a follow on patch where I intend to provide an option control for these macros separate from the pthread option.

[llvmbot]

@llvm/pr-subscribers-clang

Author: David Tenty (daltenty)

Changes

This PR does a NFC cleanup to pull the common logic for setting threadsafe macros (i.e. the pthread is present) into a helper function.

This will enable a follow on patch where I intend to provide an option control for these macros separate from the pthread option.

---

Full diff: https://github.com/llvm/llvm-project/pull/134139.diff

1 Files Affected:

• (modified) clang/lib/Basic/Targets/OSTargets.h (+47-29)

diff --git a/clang/lib/Basic/Targets/OSTargets.h b/clang/lib/Basic/Targets/OSTargets.h index a88c851797aab..0d0875a8c8d14 100644 --- a/clang/lib/Basic/Targets/OSTargets.h +++ b/clang/lib/Basic/Targets/OSTargets.h @@ -16,13 +16,16 @@ namespace clang { namespace targets { - template <typename TgtInfo> class LLVM_LIBRARY_VISIBILITY OSTargetInfo : public TgtInfo { protected: virtual void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple, MacroBuilder &Builder) const = 0; + virtual const char *getThreadSafeMacroName() const { + llvm::report_fatal_error("unimplemented for target"); + } + public: OSTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts) : TgtInfo(Triple, Opts) {} @@ -32,6 +35,13 @@ class LLVM_LIBRARY_VISIBILITY OSTargetInfo : public TgtInfo { TgtInfo::getTargetDefines(Opts, Builder); getOSDefines(Opts, TgtInfo::getTriple(), Builder); } + + void setThreadSafeDefines(const LangOptions &Opts, + MacroBuilder &Builder) const { + if (Opts.POSIXThreads) { + Builder.defineMacro(getThreadSafeMacroName()); + } + } }; void getAppleMachODefines(MacroBuilder &Builder, const LangOptions &Opts, @@ -268,12 +278,13 @@ class LLVM_LIBRARY_VISIBILITY KFreeBSDTargetInfo : public OSTargetInfo<Target> { DefineStd(Builder, "unix", Opts); Builder.defineMacro("__FreeBSD_kernel__"); Builder.defineMacro("__GLIBC__"); - if (Opts.POSIXThreads) - Builder.defineMacro("_REENTRANT"); + this->setThreadSafeDefines(Opts, Builder); if (Opts.CPlusPlus) Builder.defineMacro("_GNU_SOURCE"); } + const char *getThreadSafeMacroName() const override { return "_REENTRANT"; } + public: using OSTargetInfo<Target>::OSTargetInfo; }; @@ -321,11 +332,13 @@ class LLVM_LIBRARY_VISIBILITY HurdTargetInfo : public OSTargetInfo<Target> { Builder.defineMacro("__gnu_hurd__"); Builder.defineMacro("__MACH__"); Builder.defineMacro("__GLIBC__"); - if (Opts.POSIXThreads) - Builder.defineMacro("_REENTRANT"); + this->setThreadSafeDefines(Opts, Builder); if (Opts.CPlusPlus) Builder.defineMacro("_GNU_SOURCE"); } + + const char *getThreadSafeMacroName() const override { return "_REENTRANT"; } + public: using OSTargetInfo<Target>::OSTargetInfo; }; @@ -351,10 +364,9 @@ class LLVM_LIBRARY_VISIBILITY LinuxTargetInfo : public OSTargetInfo<Target> { Builder.defineMacro("__ANDROID_API__", "__ANDROID_MIN_SDK_VERSION__"); } } else { - Builder.defineMacro("__gnu_linux__"); + Builder.defineMacro("__gnu_linux__"); } - if (Opts.POSIXThreads) - Builder.defineMacro("_REENTRANT"); + this->setThreadSafeDefines(Opts, Builder); if (Opts.CPlusPlus) Builder.defineMacro("_GNU_SOURCE"); if (this->HasFloat128) @@ -365,6 +377,8 @@ class LLVM_LIBRARY_VISIBILITY LinuxTargetInfo : public OSTargetInfo<Target> { } } + const char *getThreadSafeMacroName() const override { return "_REENTRANT"; } + public: LinuxTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts) : OSTargetInfo<Target>(Triple, Opts) { @@ -404,12 +418,13 @@ class LLVM_LIBRARY_VISIBILITY NetBSDTargetInfo : public OSTargetInfo<Target> { // NetBSD defines; list based off of gcc output Builder.defineMacro("__NetBSD__"); Builder.defineMacro("__unix__"); - if (Opts.POSIXThreads) - Builder.defineMacro("_REENTRANT"); + this->setThreadSafeDefines(Opts, Builder); if (this->HasFloat128) Builder.defineMacro("__FLOAT128__"); } + const char *getThreadSafeMacroName() const override { return "_REENTRANT"; } + public: NetBSDTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts) : OSTargetInfo<Target>(Triple, Opts) { @@ -435,8 +450,7 @@ class LLVM_LIBRARY_VISIBILITY OpenBSDTargetInfo : public OSTargetInfo<Target> { Builder.defineMacro("__OpenBSD__"); DefineStd(Builder, "unix", Opts); - if (Opts.POSIXThreads) - Builder.defineMacro("_REENTRANT"); + this->setThreadSafeDefines(Opts, Builder); if (this->HasFloat128) Builder.defineMacro("__FLOAT128__"); @@ -444,6 +458,8 @@ class LLVM_LIBRARY_VISIBILITY OpenBSDTargetInfo : public OSTargetInfo<Target> { Builder.defineMacro("__STDC_NO_THREADS__"); } + const char *getThreadSafeMacroName() const override { return "_REENTRANT"; } + public: OpenBSDTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts) : OSTargetInfo<Target>(Triple, Opts) { @@ -634,12 +650,13 @@ class LLVM_LIBRARY_VISIBILITY SolarisTargetInfo : public OSTargetInfo<Target> { Builder.defineMacro("_LARGEFILE_SOURCE"); Builder.defineMacro("_LARGEFILE64_SOURCE"); Builder.defineMacro("__EXTENSIONS__"); - if (Opts.POSIXThreads) - Builder.defineMacro("_REENTRANT"); + this->setThreadSafeDefines(Opts, Builder); if (this->HasFloat128) Builder.defineMacro("__FLOAT128__"); } + const char *getThreadSafeMacroName() const override { return "_REENTRANT"; } + public: SolarisTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts) : OSTargetInfo<Target>(Triple, Opts) { @@ -660,8 +677,7 @@ class LLVM_LIBRARY_VISIBILITY SolarisTargetInfo : public OSTargetInfo<Target> { }; // AIX Target -template <typename Target> -class AIXTargetInfo : public OSTargetInfo<Target> { +template <typename Target> class AIXTargetInfo : public OSTargetInfo<Target> { protected: void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple, MacroBuilder &Builder) const override { @@ -712,10 +728,7 @@ class AIXTargetInfo : public OSTargetInfo<Target> { // FIXME: Do not define _LONG_LONG when -fno-long-long is specified. Builder.defineMacro("_LONG_LONG"); - if (Opts.POSIXThreads) { - Builder.defineMacro("_THREAD_SAFE"); - } - + this->setThreadSafeDefines(Opts, Builder); if (this->PointerWidth == 64) { Builder.defineMacro("__64BIT__"); } @@ -727,6 +740,8 @@ class AIXTargetInfo : public OSTargetInfo<Target> { } } + const char *getThreadSafeMacroName() const override { return "_THREAD_SAFE"; } + public: AIXTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts) : OSTargetInfo<Target>(Triple, Opts) { @@ -847,8 +862,7 @@ class LLVM_LIBRARY_VISIBILITY NaClTargetInfo : public OSTargetInfo<Target> { protected: void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple, MacroBuilder &Builder) const override { - if (Opts.POSIXThreads) - Builder.defineMacro("_REENTRANT"); + this->setThreadSafeDefines(Opts, Builder); if (Opts.CPlusPlus) Builder.defineMacro("_GNU_SOURCE"); @@ -856,6 +870,8 @@ class LLVM_LIBRARY_VISIBILITY NaClTargetInfo : public OSTargetInfo<Target> { Builder.defineMacro("__native_client__"); } + const char *getThreadSafeMacroName() const override { return "_REENTRANT"; } + public: NaClTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts) : OSTargetInfo<Target>(Triple, Opts) { @@ -896,8 +912,7 @@ class LLVM_LIBRARY_VISIBILITY FuchsiaTargetInfo : public OSTargetInfo<Target> { void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple, MacroBuilder &Builder) const override { Builder.defineMacro("__Fuchsia__"); - if (Opts.POSIXThreads) - Builder.defineMacro("_REENTRANT"); + this->setThreadSafeDefines(Opts, Builder); // Required by the libc++ locale support. if (Opts.CPlusPlus) Builder.defineMacro("_GNU_SOURCE"); @@ -906,6 +921,8 @@ class LLVM_LIBRARY_VISIBILITY FuchsiaTargetInfo : public OSTargetInfo<Target> { this->PlatformMinVersion = VersionTuple(Opts.FuchsiaAPILevel); } + const char *getThreadSafeMacroName() const override { return "_REENTRANT"; } + public: FuchsiaTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts) : OSTargetInfo<Target>(Triple, Opts) { @@ -923,8 +940,7 @@ class LLVM_LIBRARY_VISIBILITY WebAssemblyOSTargetInfo void getOSDefines(const LangOptions &Opts, const llvm::Triple &Triple, MacroBuilder &Builder) const override { // A common platform macro. - if (Opts.POSIXThreads) - Builder.defineMacro("_REENTRANT"); + this->setThreadSafeDefines(Opts, Builder); // Follow g++ convention and predefine _GNU_SOURCE for C++. if (Opts.CPlusPlus) Builder.defineMacro("_GNU_SOURCE"); @@ -932,6 +948,8 @@ class LLVM_LIBRARY_VISIBILITY WebAssemblyOSTargetInfo Builder.defineMacro("__FLOAT128__"); } + const char *getThreadSafeMacroName() const override { return "_REENTRANT"; } + public: explicit WebAssemblyOSTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts) @@ -1014,14 +1032,15 @@ class LLVM_LIBRARY_VISIBILITY OHOSTargetInfo : public OSTargetInfo<Target> { Builder.defineMacro("__LITEOS__"); } - if (Opts.POSIXThreads) - Builder.defineMacro("_REENTRANT"); + this->setThreadSafeDefines(Opts, Builder); if (Opts.CPlusPlus) Builder.defineMacro("_GNU_SOURCE"); if (this->HasFloat128) Builder.defineMacro("__FLOAT128__"); } + const char *getThreadSafeMacroName() const override { return "_REENTRANT"; } + public: OHOSTargetInfo(const llvm::Triple &Triple, const TargetOptions &Opts) : OSTargetInfo<Target>(Triple, Opts) { @@ -1041,7 +1060,6 @@ class LLVM_LIBRARY_VISIBILITY OHOSTargetInfo : public OSTargetInfo<Target> { return ".text.startup"; } }; - } // namespace targets } // namespace clang #endif // LLVM_CLANG_LIB_BASIC_TARGETS_OSTARGETS_H 

[efriedma-quic]

I don't understand what you're planning. Currently, the only thing the POSIXThreads LangOption does is control _REENTRANT or equivalent. If you provide a separate option to control the macro, the POSIXThreads option doesn't do anything at all.