glaubitz added inline comments.

================
Comment at: clang/lib/Basic/Targets/Sparc.cpp:160-164
+      // Linux doesn't need these variants, but the BSDs do.
+      if (getTriple().getOS() != llvm::Triple::Linux) {
+        Builder.defineMacro("__sparcv9");
+        Builder.defineMacro("__sparcv9__");
+      }
----------------
jrtc27 wrote:
> 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__`.
Agreed although I haven't verified whether ```__sparc_v9__``` is defined for 
NetBSD with ```-m32 -mcpu=v9```.


================
Comment at: clang/lib/Basic/Targets/Sparc.cpp:246-256
+  if (getTriple().getOS() == llvm::Triple::Linux) {
     Builder.defineMacro("__sparc_v9__");
-    Builder.defineMacro("__sparcv9__");
+  } else {
+    Builder.defineMacro("__sparcv9");
+    // Solaris doesn't need these variants, but the BSDs do.
+    if (getTriple().getOS() != llvm::Triple::Solaris) {
+      Builder.defineMacro("__sparc64__");
----------------
jrtc27 wrote:
> This doesn't need changing, we can define more things than GCC to keep it 
> simple.
Well, my original intent was to match GCC to make sure we're 100% compatible 
and I would like to keep it that way.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98574/new/

https://reviews.llvm.org/D98574

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
  • [PATCH] D98574: ... John Paul Adrian Glaubitz via Phabricator via cfe-commits
    • [PATCH] D98... Brad Smith via Phabricator via cfe-commits
    • [PATCH] D98... Jessica Clarke via Phabricator via cfe-commits
    • [PATCH] D98... John Paul Adrian Glaubitz via Phabricator via cfe-commits
    • [PATCH] D98... John Paul Adrian Glaubitz via Phabricator via cfe-commits

Reply via email to