[llvm] 51b0da7 - Recommit "[X86] Merge the FEATURE_64BIT and FEATURE_EM64T bits in X86TargetParser.def."

Hans Wennborg via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 9 09:02:24 PDT 2020


Sadly it seems this gets miscompiled by GCC 5, making Clang fail to
find the x86_64 target.

Since that's the baseline supported compiler, I've reverted in
9ecda9aa804dcb49e30e46161b29976494b0a1f9

Here are repro instructions:

$ mkdir /tmp/gcc5
$ curl https://commondatastorage.googleapis.com/chromium-browser-clang/tools/gcc530trusty.tgz
| tar -C /tmp/gcc5 -zx
$ cmake -GNinja -DCMAKE_BUILD_TYPE=Release
-DLLVM_ENABLE_PROJECTS=clang -DLLVM_TARGETS_TO_BUILD=X86
-DCMAKE_C_COMPILER=/tmp/gcc5/bin/gcc
-DCMAKE_CXX_COMPILER=/tmp/gcc5/bin/g++ ../llvm
$ ninja clang
$ touch /tmp/a.c
$  bin/clang -c /tmp/a.c
error: unknown target CPU 'x86-64'
PLEASE submit a bug report to https://bugs.llvm.org/ and include the
crash backtrace, preprocessed source, and associated run script.
Stack dump:
0.      Program arguments: bin/clang -c /tmp/a.c
bin/clang(_ZN4llvm3sys15PrintStackTraceERNS_11raw_ostreamE+0x1a)[0x18eb26a]
bin/clang(_ZN4llvm3sys17RunSignalHandlersEv+0x3a)[0x18e932a]
bin/clang(_ZN4llvm3sys15CleanupOnSignalEm+0x8a)[0x18e955a]
bin/clang[0x186a820]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x14110)[0x7f3721be6110]
/lib/x86_64-linux-gnu/libc.so.6(+0x16182c)[0x7f37217e682c]
/usr/lib/x86_64-linux-gnu/libstdc++.so.6(_ZNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEE9_M_appendEPKcm+0x3c)[0x7f3721add92c]
bin/clang(_ZN5clang10TargetInfo16CreateTargetInfoERNS_17DiagnosticsEngineERKSt10shared_ptrINS_13TargetOptionsEE+0xe9a)[0x3c4b37a]
bin/clang(_ZN5clang16CompilerInstance13ExecuteActionERNS_14FrontendActionE+0x48)[0x1feef68]
bin/clang(_ZN5clang25ExecuteCompilerInvocationEPNS_16CompilerInstanceE+0x6ca)[0x20d4a1a]
bin/clang(_Z8cc1_mainN4llvm8ArrayRefIPKcEES2_Pv+0xf9c)[0xa1e58c]
bin/clang[0xa1b627]
bin/clang[0x1ed70d5]
bin/clang(_ZN4llvm20CrashRecoveryContext9RunSafelyENS_12function_refIFvvEEE+0xa0)[0x186a9c0]
bin/clang[0x1ed7dcb]
bin/clang(_ZNK5clang6driver11Compilation14ExecuteCommandERKNS0_7CommandERPS3_+0x88)[0x1eb2938]
bin/clang(_ZNK5clang6driver11Compilation11ExecuteJobsERKNS0_7JobListERN4llvm15SmallVectorImplISt4pairIiPKNS0_7CommandEEEE+0x107)[0x1eb3037]
bin/clang(_ZN5clang6driver6Driver18ExecuteCompilationERNS0_11CompilationERN4llvm15SmallVectorImplISt4pairIiPKNS0_7CommandEEEE+0xba)[0x1ebac5a]
bin/clang(main+0x111d)[0x9a0ebd]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xeb)[0x7f37216abe0b]
bin/clang(_start+0x2a)[0xa1b07a]
clang-11: error: clang frontend command failed due to signal (use -v
to see invocation)
clang version 11.0.0 (https://github.com/llvm/llvm-project
51b0da731af75c68dd521e04cc576d5a611b1612)
Target: x86_64-unknown-linux-gnu
Thread model: posix
InstalledDir: /work/llvm.monorepo/build.foo/bin
clang-11: error: unable to execute command: Segmentation fault
clang-11: note: diagnostic msg: Error generating preprocessed source(s).

On Wed, Jul 8, 2020 at 4:04 AM Craig Topper via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
>
> Author: Craig Topper
> Date: 2020-07-07T19:01:58-07:00
> New Revision: 51b0da731af75c68dd521e04cc576d5a611b1612
>
> URL: https://github.com/llvm/llvm-project/commit/51b0da731af75c68dd521e04cc576d5a611b1612
> DIFF: https://github.com/llvm/llvm-project/commit/51b0da731af75c68dd521e04cc576d5a611b1612.diff
>
> LOG: Recommit "[X86] Merge the FEATURE_64BIT and FEATURE_EM64T bits in X86TargetParser.def."
>
> These represent the same thing but 64BIT only showed up from
> getHostCPUFeatures providing a list of featuers to clang. While
> EM64T showed up from getting the features for a named CPU.
>
> EM64T didn't have a string specifically so it would not be passed
> up to clang when getting features for a named CPU. While 64bit
> needed a name since that's how it is index.
>
> Merge them by filtering 64bit out before sending features to clang
> for named CPUs.
>
> Added:
>
>
> Modified:
>     llvm/include/llvm/Support/X86TargetParser.def
>     llvm/lib/Support/Host.cpp
>     llvm/lib/Support/X86TargetParser.cpp
>
> Removed:
>
>
>
> ################################################################################
> diff  --git a/llvm/include/llvm/Support/X86TargetParser.def b/llvm/include/llvm/Support/X86TargetParser.def
> index 9910fd615b1d..ed41295166b3 100644
> --- a/llvm/include/llvm/Support/X86TargetParser.def
> +++ b/llvm/include/llvm/Support/X86TargetParser.def
> @@ -184,10 +184,6 @@ X86_FEATURE       (CLWB,            "clwb")
>  X86_FEATURE       (CLZERO,          "clzero")
>  X86_FEATURE       (CMPXCHG16B,      "cx16")
>  X86_FEATURE       (CMPXCHG8B,       "cx8")
> -// FIXME: Merge with 64BIT? Currently separate to be used to tell if CPU is
> -// valid for 64-bit mode, but has empty string so it doesn't get added to
> -// target attributes in IR.
> -X86_FEATURE       (EM64T,           "")
>  X86_FEATURE       (ENQCMD,          "enqcmd")
>  X86_FEATURE       (F16C,            "f16c")
>  X86_FEATURE       (FSGSBASE,        "fsgsbase")
>
> diff  --git a/llvm/lib/Support/Host.cpp b/llvm/lib/Support/Host.cpp
> index 3a7d9a0242fa..db99612c97b5 100644
> --- a/llvm/lib/Support/Host.cpp
> +++ b/llvm/lib/Support/Host.cpp
> @@ -868,7 +868,7 @@ getIntelProcessorTypeAndSubtype(unsigned Family, unsigned Model,
>          }
>          break;
>        }
> -      if (testFeature(X86::FEATURE_EM64T)) {
> +      if (testFeature(X86::FEATURE_64BIT)) {
>          *Type = X86::INTEL_CORE2; // "core2"
>          *Subtype = X86::INTEL_CORE2_65;
>          break;
> @@ -894,7 +894,7 @@ getIntelProcessorTypeAndSubtype(unsigned Family, unsigned Model,
>      }
>      break;
>    case 15: {
> -    if (testFeature(X86::FEATURE_EM64T)) {
> +    if (testFeature(X86::FEATURE_64BIT)) {
>        *Type = X86::INTEL_NOCONA;
>        break;
>      }
> @@ -1140,7 +1140,7 @@ static void getAvailableFeatures(unsigned ECX, unsigned EDX, unsigned MaxLeaf,
>      setFeature(X86::FEATURE_FMA4);
>
>    if (HasExtLeaf1 && ((EDX >> 29) & 1))
> -    setFeature(X86::FEATURE_EM64T);
> +    setFeature(X86::FEATURE_64BIT);
>  }
>
>  StringRef sys::getHostCPUName() {
>
> diff  --git a/llvm/lib/Support/X86TargetParser.cpp b/llvm/lib/Support/X86TargetParser.cpp
> index df03f63e720e..cbb7f6186d0d 100644
> --- a/llvm/lib/Support/X86TargetParser.cpp
> +++ b/llvm/lib/Support/X86TargetParser.cpp
> @@ -48,6 +48,14 @@ class FeatureBitset {
>      return (Bits[I / 32] & Mask) != 0;
>    }
>
> +  constexpr FeatureBitset &operator&=(const FeatureBitset &RHS) {
> +    for (unsigned I = 0, E = array_lengthof(Bits); I != E; ++I) {
> +      uint32_t NewBits = Bits[I] & RHS.Bits[I];
> +      Bits[I] = NewBits;
> +    }
> +    return *this;
> +  }
> +
>    constexpr FeatureBitset &operator|=(const FeatureBitset &RHS) {
>      for (unsigned I = 0, E = array_lengthof(Bits); I != E; ++I) {
>        uint32_t NewBits = Bits[I] | RHS.Bits[I];
> @@ -57,16 +65,14 @@ class FeatureBitset {
>    }
>
>    constexpr FeatureBitset operator&(const FeatureBitset &RHS) const {
> -    FeatureBitset Result;
> -    for (unsigned I = 0, E = array_lengthof(Bits); I != E; ++I)
> -      Result.Bits[I] = Bits[I] & RHS.Bits[I];
> +    FeatureBitset Result = *this;
> +    Result &= RHS;
>      return Result;
>    }
>
>    constexpr FeatureBitset operator|(const FeatureBitset &RHS) const {
> -    FeatureBitset Result;
> -    for (unsigned I = 0, E = array_lengthof(Bits); I != E; ++I)
> -      Result.Bits[I] = Bits[I] | RHS.Bits[I];
> +    FeatureBitset Result = *this;
> +    Result |= RHS;
>      return Result;
>    }
>
> @@ -111,10 +117,10 @@ static constexpr FeatureBitset FeaturesPentium4 =
>  static constexpr FeatureBitset FeaturesPrescott =
>      FeaturesPentium4 | FeatureSSE3;
>  static constexpr FeatureBitset FeaturesNocona =
> -    FeaturesPrescott | FeatureEM64T | FeatureCMPXCHG16B;
> +    FeaturesPrescott | Feature64BIT | FeatureCMPXCHG16B;
>
>  // Basic 64-bit capable CPU.
> -static constexpr FeatureBitset FeaturesX86_64 = FeaturesPentium4 | FeatureEM64T;
> +static constexpr FeatureBitset FeaturesX86_64 = FeaturesPentium4 | Feature64BIT;
>
>  // Intel Core CPUs
>  static constexpr FeatureBitset FeaturesCore2 =
> @@ -201,7 +207,7 @@ static constexpr FeatureBitset FeaturesAthlon =
>  static constexpr FeatureBitset FeaturesAthlonXP =
>      FeaturesAthlon | FeatureFXSR | FeatureSSE;
>  static constexpr FeatureBitset FeaturesK8 =
> -    FeaturesAthlonXP | FeatureSSE2 | FeatureEM64T;
> +    FeaturesAthlonXP | FeatureSSE2 | Feature64BIT;
>  static constexpr FeatureBitset FeaturesK8SSE3 = FeaturesK8 | FeatureSSE3;
>  static constexpr FeatureBitset FeaturesAMDFAM10 =
>      FeaturesK8SSE3 | FeatureCMPXCHG16B | FeatureLZCNT | FeaturePOPCNT |
> @@ -209,7 +215,7 @@ static constexpr FeatureBitset FeaturesAMDFAM10 =
>
>  // Bobcat architecture processors.
>  static constexpr FeatureBitset FeaturesBTVER1 =
> -    FeatureX87 | FeatureCMPXCHG8B | FeatureCMPXCHG16B | FeatureEM64T |
> +    FeatureX87 | FeatureCMPXCHG8B | FeatureCMPXCHG16B | Feature64BIT |
>      FeatureFXSR | FeatureLZCNT | FeatureMMX | FeaturePOPCNT | FeaturePRFCHW |
>      FeatureSSE | FeatureSSE2 | FeatureSSE3 | FeatureSSSE3 | FeatureSSE4_A |
>      FeatureSAHF;
> @@ -220,7 +226,7 @@ static constexpr FeatureBitset FeaturesBTVER2 =
>  // AMD Bulldozer architecture processors.
>  static constexpr FeatureBitset FeaturesBDVER1 =
>      FeatureX87 | FeatureAES | FeatureAVX | FeatureCMPXCHG8B |
> -    FeatureCMPXCHG16B | FeatureEM64T | FeatureFMA4 | FeatureFXSR | FeatureLWP |
> +    FeatureCMPXCHG16B | Feature64BIT | FeatureFMA4 | FeatureFXSR | FeatureLWP |
>      FeatureLZCNT | FeatureMMX | FeaturePCLMUL | FeaturePOPCNT | FeaturePRFCHW |
>      FeatureSAHF | FeatureSSE | FeatureSSE2 | FeatureSSE3 | FeatureSSSE3 |
>      FeatureSSE4_1 | FeatureSSE4_2 | FeatureSSE4_A | FeatureXOP | FeatureXSAVE;
> @@ -236,7 +242,7 @@ static constexpr FeatureBitset FeaturesBDVER4 =
>  static constexpr FeatureBitset FeaturesZNVER1 =
>      FeatureX87 | FeatureADX | FeatureAES | FeatureAVX | FeatureAVX2 |
>      FeatureBMI | FeatureBMI2 | FeatureCLFLUSHOPT | FeatureCLZERO |
> -    FeatureCMPXCHG8B | FeatureCMPXCHG16B | FeatureEM64T | FeatureF16C |
> +    FeatureCMPXCHG8B | FeatureCMPXCHG16B | Feature64BIT | FeatureF16C |
>      FeatureFMA | FeatureFSGSBASE | FeatureFXSR | FeatureLZCNT | FeatureMMX |
>      FeatureMOVBE | FeatureMWAITX | FeaturePCLMUL | FeaturePOPCNT |
>      FeaturePRFCHW | FeatureRDRND | FeatureRDSEED | FeatureSAHF | FeatureSHA |
> @@ -363,7 +369,7 @@ static constexpr ProcInfo Processors[] = {
>
>  X86::CPUKind llvm::X86::parseArchX86(StringRef CPU, bool Only64Bit) {
>    for (const auto &P : Processors)
> -    if (P.Name == CPU && (P.Features[FEATURE_EM64T] || !Only64Bit))
> +    if (P.Name == CPU && (P.Features[FEATURE_64BIT] || !Only64Bit))
>        return P.Kind;
>
>    return CK_None;
> @@ -372,7 +378,7 @@ X86::CPUKind llvm::X86::parseArchX86(StringRef CPU, bool Only64Bit) {
>  void llvm::X86::fillValidCPUArchList(SmallVectorImpl<StringRef> &Values,
>                                       bool Only64Bit) {
>    for (const auto &P : Processors)
> -    if (!P.Name.empty() && (P.Features[FEATURE_EM64T] || !Only64Bit))
> +    if (!P.Name.empty() && (P.Features[FEATURE_64BIT] || !Only64Bit))
>        Values.emplace_back(P.Name);
>  }
>
> @@ -401,7 +407,6 @@ static constexpr FeatureBitset ImpliedFeaturesCLZERO = {};
>  static constexpr FeatureBitset ImpliedFeaturesCMOV = {};
>  static constexpr FeatureBitset ImpliedFeaturesCMPXCHG16B = {};
>  static constexpr FeatureBitset ImpliedFeaturesCMPXCHG8B = {};
> -static constexpr FeatureBitset ImpliedFeaturesEM64T = {};
>  static constexpr FeatureBitset ImpliedFeaturesENQCMD = {};
>  static constexpr FeatureBitset ImpliedFeaturesFSGSBASE = {};
>  static constexpr FeatureBitset ImpliedFeaturesFXSR = {};
> @@ -528,8 +533,14 @@ void llvm::X86::getFeaturesForCPU(StringRef CPU,
>                           [&](const ProcInfo &P) { return P.Name == CPU; });
>    assert(I != std::end(Processors) && "Processor not found!");
>
> +  FeatureBitset Bits = I->Features;
> +
> +  // Remove the 64-bit feature which we only use to validate if a CPU can
> +  // be used with 64-bit mode.
> +  Bits &= ~Feature64BIT;
> +
>    // Add the string version of all set bits.
> -  getFeatureBitsAsStrings(I->Features, EnabledFeatures);
> +  getFeatureBitsAsStrings(Bits, EnabledFeatures);
>  }
>
>  // For each feature that is (transitively) implied by this feature, set it.
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list