[PATCH] D42154: Don't generate inline atomics for i386/i486

[Wei Mi via Phabricator via cfe-commits]

wmi added a comment.

In https://reviews.llvm.org/D42154#977975, @efriedma wrote:

> The LLVM backend currently assumes every CPU is Pentium-compatible.  If we're going to change that in clang, we should probably fix the backend as well.


With the patch, for i386/i486 targets, clang will generate more atomic libcalls than before, for which llvm backend will not do anything extra, so no fix is necessary in llvm backend for the patch to work.



================
Comment at: lib/Basic/Targets/X86.h:472
+    CPUKind Kind = getCPUKind(Opts.CPU);
+    if (Kind >= CK_i586 || Kind == CK_Generic)
+      MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 64;
----------------
craig.topper wrote:
> craig.topper wrote:
> > efriedma wrote:
> > > What exactly does "CK_Generic" mean here?  Under what circumstances does it show up? We should have a specific default for every supported target, if user doesn't specify a CPU with -march.
> > CK_Generic is the default for 32-bit mode with no -march. CK_x86_64 is the default for 64-bit mode with no march.
> > 
> > I'm not sure we can assume CK_Generic is 586 or better. We don't assume that in the preprocessor defines.
> > 
> > ```
> >   if (CPU >= CK_i486) {
> >     Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_1");
> >     Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_2");
> >     Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_4");
> >   }
> >   if (CPU >= CK_i586)
> >     Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_8");
> >   if (HasCX16)
> >     Builder.defineMacro("__GCC_HAVE_SYNC_COMPARE_AND_SWAP_16");
> > ```
> Maybe we don't default to "generic" anywhere.  Its very platform specific. getX86TargetCPU in lib/Driver/ToolChains/Arch/X86.cpp contains the magic I think.
Like the command "clang -cc1 -triple=i686-linux-gnu", the cpu will be set to CK_Generic. I want to set the MaxAtomicInlineWidth of the default target to 64 (same as before) to minimize the churn. In the patch, MaxAtomicInlineWidth will only be changed if i386/i486 are explicitly specified.


Repository:
  rC Clang

https://reviews.llvm.org/D42154