[PATCH] D86621: [clang][Sparc] Default to -mcpu=v9 for SparcV8 on Solaris

Mon Aug 31 10:54:33 PDT 2020

efriedma added a comment.

> While the fix proper is trivial: just two lines in lib/Driver/ToolChains/CommonArgs.cpp, finding the right place has been nightmarishly difficult: I'd have expected handling of a Solaris/SPARC CPU default in either of Solaris or SPARC specific files, but not deeply hidden in common code. I've come across issues like this over and over again: configuration information in LLVM is spread all over the place, difficult to find or just to know that it exists.

The clang driver is messy, yes.  Other places are pretty good about this, mostly.

For compiler-rt, the XFAILs should probably reflect whatever config the bot is running.  (Alternatively, you could use UNSUPPORTED, but that doesn't seem warranted here.)

================
Comment at: clang/lib/Basic/Targets/Sparc.cpp:224
+    Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8");
+  }
 }
----------------
This probably should be refactored so the target-independent code generates it based on MaxAtomicInlineWidth, instead of duplicating it for each target.  But I guess you don't need to do that here.

>From the other code, the `getCPUGeneration(CPU) == CG_V9` check should only guard the definition of __GCC_HAVE_SYNC_COMPARE_AND_SWAP_8?

================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:350
       return A->getValue();
+    if (T.getArch() == llvm::Triple::sparc && T.isOSSolaris())
+      return "v9";
----------------
Do we want to make sparc and sparcel behave differently here?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86621