Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Frontend: sink vendor definitions from Basic to Frontend #80364

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

compnerd
Copy link
Member

@compnerd compnerd commented Feb 1, 2024

The vendor specific macro definitions are based on the vendor rather than the target specific information. This ensures that __APPLE__ is always defined for *-apple-* targets. Take the opportunity to do likewise with the __AMD__ vendor macro.

@compnerd
Copy link
Member Author

compnerd commented Feb 1, 2024

CC: @kubamracek

@llvmbot llvmbot added clang Clang issues not falling into any other category backend:AMDGPU clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Feb 1, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Feb 1, 2024

@llvm/pr-subscribers-backend-amdgpu

@llvm/pr-subscribers-clang

Author: Saleem Abdulrasool (compnerd)

Changes

The vendor specific macro definitions are based on the vendor rather than the target specific information. This ensures that __APPLE__ is always defined for *-apple-* targets. Take the opportunity to do likewise with the __AMD__ vendor macro.


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

3 Files Affected:

  • (modified) clang/lib/Basic/Targets/AMDGPU.cpp (-1)
  • (modified) clang/lib/Basic/Targets/OSTargets.cpp (-1)
  • (modified) clang/lib/Frontend/InitPreprocessor.cpp (+5)
diff --git a/clang/lib/Basic/Targets/AMDGPU.cpp b/clang/lib/Basic/Targets/AMDGPU.cpp
index 141501e8a4d9a..c6d68f591a5a8 100644
--- a/clang/lib/Basic/Targets/AMDGPU.cpp
+++ b/clang/lib/Basic/Targets/AMDGPU.cpp
@@ -266,7 +266,6 @@ ArrayRef<Builtin::Info> AMDGPUTargetInfo::getTargetBuiltins() const {
 
 void AMDGPUTargetInfo::getTargetDefines(const LangOptions &Opts,
                                         MacroBuilder &Builder) const {
-  Builder.defineMacro("__AMD__");
   Builder.defineMacro("__AMDGPU__");
 
   if (isAMDGCN(getTriple()))
diff --git a/clang/lib/Basic/Targets/OSTargets.cpp b/clang/lib/Basic/Targets/OSTargets.cpp
index 899aefa6173ac..64b4514ba55bb 100644
--- a/clang/lib/Basic/Targets/OSTargets.cpp
+++ b/clang/lib/Basic/Targets/OSTargets.cpp
@@ -23,7 +23,6 @@ void getDarwinDefines(MacroBuilder &Builder, const LangOptions &Opts,
                       const llvm::Triple &Triple, StringRef &PlatformName,
                       VersionTuple &PlatformMinVersion) {
   Builder.defineMacro("__APPLE_CC__", "6000");
-  Builder.defineMacro("__APPLE__");
   Builder.defineMacro("__STDC_NO_THREADS__");
 
   // AddressSanitizer doesn't play well with source fortification, which is on
diff --git a/clang/lib/Frontend/InitPreprocessor.cpp b/clang/lib/Frontend/InitPreprocessor.cpp
index 877e205e2e9bf..27621ee332419 100644
--- a/clang/lib/Frontend/InitPreprocessor.cpp
+++ b/clang/lib/Frontend/InitPreprocessor.cpp
@@ -804,6 +804,11 @@ static void InitializePredefinedMacros(const TargetInfo &TI,
     }
   }
 
+  if (TI.getTriple().getVendor() == Triple::AMD)
+    Builder.defineMacro("__AMD__");
+  if (TI.getTriple().getVendor() == Triple::Apple)
+    Builder.defineMacro("__APPLE__");
+
   // Define macros for the C11 / C++11 memory orderings
   Builder.defineMacro("__ATOMIC_RELAXED", "0");
   Builder.defineMacro("__ATOMIC_CONSUME", "1");

if (TI.getTriple().getVendor() == Triple::AMD)
Builder.defineMacro("__AMD__");
if (TI.getTriple().getVendor() == Triple::Apple)
Builder.defineMacro("__APPLE__");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is a behavior change for non-darwin triples, but only in a "tree falls in the forest" kind of way.

@fpetrogalli you were talking about this a while back. I don't remember the verdict.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, and something that I think that is desirable in the context of Swift as well (where we are looking at some of the embedded system usage where the target triple may be something like arm64-apple-none-macho).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

The vendor specific macro definitions are based on the vendor rather
than the target specific information. This ensures that `__APPLE__` is
always defined for `*-apple-*` targets. Take the opportunity to do
likewise with the `__AMD__` vendor macro.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend:AMDGPU clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants