[PATCH] D38824: [X86] Synchronize the CPU predefined macros with gcc
craig.topper updated this revision to Diff 118973. craig.topper added a comment. Only define "corei7" on nehalem/westmere to match gcc. Don't define anything for the CPUs newer than that. Add comments to the CPUs where gcc has two sets of defines and we have only one. https://reviews.llvm.org/D38824 Files: lib/Basic/Targets/X86.cpp test/Preprocessor/predefined-arch-macros.c Index: test/Preprocessor/predefined-arch-macros.c === --- test/Preprocessor/predefined-arch-macros.c +++ test/Preprocessor/predefined-arch-macros.c @@ -427,11 +427,8 @@ // CHECK_COREI7_AVX_M32: #define __SSSE3__ 1 // CHECK_COREI7_AVX_M32: #define __XSAVEOPT__ 1 // CHECK_COREI7_AVX_M32: #define __XSAVE__ 1 -// CHECK_COREI7_AVX_M32: #define __corei7 1 -// CHECK_COREI7_AVX_M32: #define __corei7__ 1 // CHECK_COREI7_AVX_M32: #define __i386 1 // CHECK_COREI7_AVX_M32: #define __i386__ 1 -// CHECK_COREI7_AVX_M32: #define __tune_corei7__ 1 // CHECK_COREI7_AVX_M32: #define i386 1 // RUN: %clang -march=corei7-avx -m64 -E -dM %s -o - 2>&1 \ // RUN: -target i386-unknown-linux \ @@ -454,9 +451,6 @@ // CHECK_COREI7_AVX_M64: #define __XSAVE__ 1 // CHECK_COREI7_AVX_M64: #define __amd64 1 // CHECK_COREI7_AVX_M64: #define __amd64__ 1 -// CHECK_COREI7_AVX_M64: #define __corei7 1 -// CHECK_COREI7_AVX_M64: #define __corei7__ 1 -// CHECK_COREI7_AVX_M64: #define __tune_corei7__ 1 // CHECK_COREI7_AVX_M64: #define __x86_64 1 // CHECK_COREI7_AVX_M64: #define __x86_64__ 1 // @@ -477,11 +471,8 @@ // CHECK_CORE_AVX_I_M32: #define __SSSE3__ 1 // CHECK_CORE_AVX_I_M32: #define __XSAVEOPT__ 1 // CHECK_CORE_AVX_I_M32: #define __XSAVE__ 1 -// CHECK_CORE_AVX_I_M32: #define __corei7 1 -// CHECK_CORE_AVX_I_M32: #define __corei7__ 1 // CHECK_CORE_AVX_I_M32: #define __i386 1 // CHECK_CORE_AVX_I_M32: #define __i386__ 1 -// CHECK_CORE_AVX_I_M32: #define __tune_corei7__ 1 // CHECK_CORE_AVX_I_M32: #define i386 1 // RUN: %clang -march=core-avx-i -m64 -E -dM %s -o - 2>&1 \ // RUN: -target i386-unknown-linux \ @@ -504,9 +495,6 @@ // CHECK_CORE_AVX_I_M64: #define __XSAVE__ 1 // CHECK_CORE_AVX_I_M64: #define __amd64 1 // CHECK_CORE_AVX_I_M64: #define __amd64__ 1 -// CHECK_CORE_AVX_I_M64: #define __corei7 1 -// CHECK_CORE_AVX_I_M64: #define __corei7__ 1 -// CHECK_CORE_AVX_I_M64: #define __tune_corei7__ 1 // CHECK_CORE_AVX_I_M64: #define __x86_64 1 // CHECK_CORE_AVX_I_M64: #define __x86_64__ 1 // @@ -533,11 +521,8 @@ // CHECK_CORE_AVX2_M32: #define __SSSE3__ 1 // CHECK_CORE_AVX2_M32: #define __XSAVEOPT__ 1 // CHECK_CORE_AVX2_M32: #define __XSAVE__ 1 -// CHECK_CORE_AVX2_M32: #define __corei7 1 -// CHECK_CORE_AVX2_M32: #define __corei7__ 1 // CHECK_CORE_AVX2_M32: #define __i386 1 // CHECK_CORE_AVX2_M32: #define __i386__ 1 -// CHECK_CORE_AVX2_M32: #define __tune_corei7__ 1 // CHECK_CORE_AVX2_M32: #define i386 1 // RUN: %clang -march=core-avx2 -m64 -E -dM %s -o - 2>&1 \ // RUN: -target i386-unknown-linux \ @@ -566,9 +551,6 @@ // CHECK_CORE_AVX2_M64: #define __XSAVE__ 1 // CHECK_CORE_AVX2_M64: #define __amd64 1 // CHECK_CORE_AVX2_M64: #define __amd64__ 1 -// CHECK_CORE_AVX2_M64: #define __corei7 1 -// CHECK_CORE_AVX2_M64: #define __corei7__ 1 -// CHECK_CORE_AVX2_M64: #define __tune_corei7__ 1 // CHECK_CORE_AVX2_M64: #define __x86_64 1 // CHECK_CORE_AVX2_M64: #define __x86_64__ 1 // @@ -597,11 +579,8 @@ // CHECK_BROADWELL_M32: #define __SSSE3__ 1 // CHECK_BROADWELL_M32: #define __XSAVEOPT__ 1 // CHECK_BROADWELL_M32: #define __XSAVE__ 1 -// CHECK_BROADWELL_M32: #define __corei7 1 -// CHECK_BROADWELL_M32: #define __corei7__ 1 // CHECK_BROADWELL_M32: #define __i386 1 // CHECK_BROADWELL_M32: #define __i386__ 1 -// CHECK_BROADWELL_M32: #define __tune_corei7__ 1 // CHECK_BROADWELL_M32: #define i386 1 // RUN: %clang -march=broadwell -m64 -E -dM %s -o - 2>&1 \ // RUN: -target i386-unknown-linux \ @@ -632,9 +611,6 @@ // CHECK_BROADWELL_M64: #define __XSAVE__ 1 // CHECK_BROADWELL_M64: #define __amd64 1 // CHECK_BROADWELL_M64: #define __amd64__ 1 -// CHECK_BROADWELL_M64: #define __corei7 1 -// CHECK_BROADWELL_M64: #define __corei7__ 1 -// CHECK_BROADWELL_M64: #define __tune_corei7__ 1 // CHECK_BROADWELL_M64: #define __x86_64 1 // CHECK_BROADWELL_M64: #define __x86_64__ 1 // @@ -890,9 +866,6 @@ // CHECK_SKX_M32: #define __XSAVE__ 1 // CHECK_SKX_M32: #define __i386 1 // CHECK_SKX_M32: #define __i386__ 1 -// CHECK_SKX_M32: #define __skx 1 -// CHECK_SKX_M32: #define __skx__ 1 -// CHECK_SKX_M32: #define __tune_skx__ 1 // CHECK_SKX_M32: #define i386 1 // RUN: %clang -march=skylake-avx512 -m64 -E -dM %s -o - 2>&1 \ @@ -934,9 +907,6 @@ // CHECK_SKX_M64: #define __XSAVE__ 1 // CHECK_SKX_M64: #define __amd64 1 // CHECK_SKX_M64: #define __amd64__ 1 -// CHECK_SKX_M64: #define __skx 1 -// CHECK_SKX_M64: #define __skx__ 1 -// CHECK_SKX_M64: #define __tune_skx__ 1 // CHECK_SKX_M64: #define __x86_64 1 // CHECK_SKX_M64: #define __x86_64__ 1 // Index: lib/Basic/Tar
[PATCH] D38824: [X86] Synchronize the CPU predefined macros with gcc
chandlerc added inline comments. Comment at: lib/Basic/Targets/X86.cpp:844-845 -// FIXME: Historically, we defined this legacy name, it would be nice to -// remove it at some point. We've never exposed fine-grained names for -// recent primary x86 CPUs, and we should keep it that way. -defineCPUMacros(Builder, "corei7"); craig.topper wrote: > chandlerc wrote: > > This seems to undo the idea that we should keep avoiding exposing > > fine-grained CPU names? What's new that changes this? > CPUs newer than the ones with that comment seem to have ignored said comment. > > Probably be cause we don't have a definition for what to do for new CPUs if > we aren't going to expose fine grained names. Do we just call everything > corei7 forever? My hope would have been that people use *ISA*-based macros rather than anything w/ a specific CPU. So I would have *removed* the corei7 for future CPUs and simply not defined anything for them at all. I think exposing something beyond ISA featureset through preprocessor macros is questionable at best. I would at least want to see concrete use cases first. Comment at: lib/Basic/Targets/X86.cpp:852 +defineCPUMacros(Builder, "core_avx2"); +defineCPUMacros(Builder, "haswell"); break; craig.topper wrote: > chandlerc wrote: > > I find calling a Westmere CPU `nehalem` a little odd. Calling IvyBridge a > > `sandybridge' CPU seems quite confusing. But calling Skylake (client) and > > Cannonlake (all? client?) `haswell` seems deeply weird. > This implementation matches what gcc does. I agree its weird. > > gcc doesn't implement cannonlake yet so i don't know what they'll do. Ok, but maybe we should ask GCC to stop doing this. ;] We could talk to them and try to figure out the right strategy is. My proposed strategy (see other comment) is to restrict macros to ISA facilities. https://reviews.llvm.org/D38824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38824: [X86] Synchronize the CPU predefined macros with gcc
craig.topper added inline comments. Comment at: lib/Basic/Targets/X86.cpp:844-845 -// FIXME: Historically, we defined this legacy name, it would be nice to -// remove it at some point. We've never exposed fine-grained names for -// recent primary x86 CPUs, and we should keep it that way. -defineCPUMacros(Builder, "corei7"); chandlerc wrote: > This seems to undo the idea that we should keep avoiding exposing > fine-grained CPU names? What's new that changes this? CPUs newer than the ones with that comment seem to have ignored said comment. Probably be cause we don't have a definition for what to do for new CPUs if we aren't going to expose fine grained names. Do we just call everything corei7 forever? Comment at: lib/Basic/Targets/X86.cpp:852 +defineCPUMacros(Builder, "core_avx2"); +defineCPUMacros(Builder, "haswell"); break; chandlerc wrote: > I find calling a Westmere CPU `nehalem` a little odd. Calling IvyBridge a > `sandybridge' CPU seems quite confusing. But calling Skylake (client) and > Cannonlake (all? client?) `haswell` seems deeply weird. This implementation matches what gcc does. I agree its weird. gcc doesn't implement cannonlake yet so i don't know what they'll do. https://reviews.llvm.org/D38824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38824: [X86] Synchronize the CPU predefined macros with gcc
chandlerc added inline comments. Comment at: lib/Basic/Targets/X86.cpp:844-845 -// FIXME: Historically, we defined this legacy name, it would be nice to -// remove it at some point. We've never exposed fine-grained names for -// recent primary x86 CPUs, and we should keep it that way. -defineCPUMacros(Builder, "corei7"); This seems to undo the idea that we should keep avoiding exposing fine-grained CPU names? What's new that changes this? Comment at: lib/Basic/Targets/X86.cpp:852 +defineCPUMacros(Builder, "core_avx2"); +defineCPUMacros(Builder, "haswell"); break; I find calling a Westmere CPU `nehalem` a little odd. Calling IvyBridge a `sandybridge' CPU seems quite confusing. But calling Skylake (client) and Cannonlake (all? client?) `haswell` seems deeply weird. https://reviews.llvm.org/D38824 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D38824: [X86] Synchronize the CPU predefined macros with gcc
craig.topper created this revision. Herald added a subscriber: krytarowski. We were using corei7 for a large swatch of Intel CPUs. gcc has a different defines that more closely match the march flags. This updates to match. It also fixes skylake-avx512 and adds silvermont in addition to slm. https://reviews.llvm.org/D38824 Files: lib/Basic/Targets/X86.cpp test/Preprocessor/predefined-arch-macros.c Index: test/Preprocessor/predefined-arch-macros.c === --- test/Preprocessor/predefined-arch-macros.c +++ test/Preprocessor/predefined-arch-macros.c @@ -427,11 +427,14 @@ // CHECK_COREI7_AVX_M32: #define __SSSE3__ 1 // CHECK_COREI7_AVX_M32: #define __XSAVEOPT__ 1 // CHECK_COREI7_AVX_M32: #define __XSAVE__ 1 -// CHECK_COREI7_AVX_M32: #define __corei7 1 -// CHECK_COREI7_AVX_M32: #define __corei7__ 1 +// CHECK_COREI7_AVX_M32: #define __corei7_avx 1 +// CHECK_COREI7_AVX_M32: #define __corei7_avx__ 1 // CHECK_COREI7_AVX_M32: #define __i386 1 // CHECK_COREI7_AVX_M32: #define __i386__ 1 -// CHECK_COREI7_AVX_M32: #define __tune_corei7__ 1 +// CHECK_COREI7_AVX_M32: #define __sandybridge 1 +// CHECK_COREI7_AVX_M32: #define __sandybridge__ 1 +// CHECK_COREI7_AVX_M32: #define __tune_corei7_avx__ 1 +// CHECK_COREI7_AVX_M32: #define __tune_sandybridge__ 1 // CHECK_COREI7_AVX_M32: #define i386 1 // RUN: %clang -march=corei7-avx -m64 -E -dM %s -o - 2>&1 \ // RUN: -target i386-unknown-linux \ @@ -454,9 +457,12 @@ // CHECK_COREI7_AVX_M64: #define __XSAVE__ 1 // CHECK_COREI7_AVX_M64: #define __amd64 1 // CHECK_COREI7_AVX_M64: #define __amd64__ 1 -// CHECK_COREI7_AVX_M64: #define __corei7 1 -// CHECK_COREI7_AVX_M64: #define __corei7__ 1 -// CHECK_COREI7_AVX_M64: #define __tune_corei7__ 1 +// CHECK_COREI7_AVX_M64: #define __corei7_avx 1 +// CHECK_COREI7_AVX_M64: #define __corei7_avx__ 1 +// CHECK_COREI7_AVX_M64: #define __sandybridge 1 +// CHECK_COREI7_AVX_M64: #define __sandybridge__ 1 +// CHECK_COREI7_AVX_M64: #define __tune_corei7_avx__ 1 +// CHECK_COREI7_AVX_M64: #define __tune_sandybridge__ 1 // CHECK_COREI7_AVX_M64: #define __x86_64 1 // CHECK_COREI7_AVX_M64: #define __x86_64__ 1 // @@ -477,11 +483,14 @@ // CHECK_CORE_AVX_I_M32: #define __SSSE3__ 1 // CHECK_CORE_AVX_I_M32: #define __XSAVEOPT__ 1 // CHECK_CORE_AVX_I_M32: #define __XSAVE__ 1 -// CHECK_CORE_AVX_I_M32: #define __corei7 1 -// CHECK_CORE_AVX_I_M32: #define __corei7__ 1 +// CHECK_CORE_AVX_I_M32: #define __corei7_avx 1 +// CHECK_CORE_AVX_I_M32: #define __corei7_avx__ 1 // CHECK_CORE_AVX_I_M32: #define __i386 1 // CHECK_CORE_AVX_I_M32: #define __i386__ 1 -// CHECK_CORE_AVX_I_M32: #define __tune_corei7__ 1 +// CHECK_CORE_AVX_I_M32: #define __sandybridge 1 +// CHECK_CORE_AVX_I_M32: #define __sandybridge__ 1 +// CHECK_CORE_AVX_I_M32: #define __tune_corei7_avx__ 1 +// CHECK_CORE_AVX_I_M32: #define __tune_sandybridge__ 1 // CHECK_CORE_AVX_I_M32: #define i386 1 // RUN: %clang -march=core-avx-i -m64 -E -dM %s -o - 2>&1 \ // RUN: -target i386-unknown-linux \ @@ -504,9 +513,12 @@ // CHECK_CORE_AVX_I_M64: #define __XSAVE__ 1 // CHECK_CORE_AVX_I_M64: #define __amd64 1 // CHECK_CORE_AVX_I_M64: #define __amd64__ 1 -// CHECK_CORE_AVX_I_M64: #define __corei7 1 -// CHECK_CORE_AVX_I_M64: #define __corei7__ 1 -// CHECK_CORE_AVX_I_M64: #define __tune_corei7__ 1 +// CHECK_CORE_AVX_I_M64: #define __corei7_avx 1 +// CHECK_CORE_AVX_I_M64: #define __corei7_avx__ 1 +// CHECK_CORE_AVX_I_M64: #define __sandybridge 1 +// CHECK_CORE_AVX_I_M64: #define __sandybridge__ 1 +// CHECK_CORE_AVX_I_M64: #define __tune_corei7_avx__ 1 +// CHECK_CORE_AVX_I_M64: #define __tune_sandybridge__ 1 // CHECK_CORE_AVX_I_M64: #define __x86_64 1 // CHECK_CORE_AVX_I_M64: #define __x86_64__ 1 // @@ -533,11 +545,14 @@ // CHECK_CORE_AVX2_M32: #define __SSSE3__ 1 // CHECK_CORE_AVX2_M32: #define __XSAVEOPT__ 1 // CHECK_CORE_AVX2_M32: #define __XSAVE__ 1 -// CHECK_CORE_AVX2_M32: #define __corei7 1 -// CHECK_CORE_AVX2_M32: #define __corei7__ 1 +// CHECK_CORE_AVX2_M32: #define __core_avx2 1 +// CHECK_CORE_AVX2_M32: #define __core_avx2__ 1 +// CHECK_CORE_AVX2_M32: #define __haswell 1 +// CHECK_CORE_AVX2_M32: #define __haswell__ 1 // CHECK_CORE_AVX2_M32: #define __i386 1 // CHECK_CORE_AVX2_M32: #define __i386__ 1 -// CHECK_CORE_AVX2_M32: #define __tune_corei7__ 1 +// CHECK_CORE_AVX2_M32: #define __tune_core_avx2__ 1 +// CHECK_CORE_AVX2_M32: #define __tune_haswell__ 1 // CHECK_CORE_AVX2_M32: #define i386 1 // RUN: %clang -march=core-avx2 -m64 -E -dM %s -o - 2>&1 \ // RUN: -target i386-unknown-linux \ @@ -566,9 +581,12 @@ // CHECK_CORE_AVX2_M64: #define __XSAVE__ 1 // CHECK_CORE_AVX2_M64: #define __amd64 1 // CHECK_CORE_AVX2_M64: #define __amd64__ 1 -// CHECK_CORE_AVX2_M64: #define __corei7 1 -// CHECK_CORE_AVX2_M64: #define __corei7__ 1 -// CHECK_CORE_AVX2_M64: #define __tune_corei7__ 1 +// CHECK_CORE_AVX2_M64: #define __core_avx2 1 +// CHECK_CORE_AVX2_M64: #define __core_avx2__ 1 +// CHECK_CORE_AVX2_M64: #def
