[llvm] b2c65be - [X86] Rework the "sahf" feature flag to only apply to 64-bit mode.

[Craig Topper via llvm-commits]

Author: Craig Topper
Date: 2020-07-22T16:57:46-07:00
New Revision: b2c65beb14b5983a09e6eb4df8d41f5ddba1d6eb

URL: https://github.com/llvm/llvm-project/commit/b2c65beb14b5983a09e6eb4df8d41f5ddba1d6eb
DIFF: https://github.com/llvm/llvm-project/commit/b2c65beb14b5983a09e6eb4df8d41f5ddba1d6eb.diff

LOG: [X86] Rework the "sahf" feature flag to only apply to 64-bit mode.

SAHF/LAHF instructions are always available in 32-bit mode. Early
64-bit capable CPUs made the undefined opcodes in 64-bit mode. This
was changed on later CPUs.

We have a feature flag to control our usage of these instructions.
This feature flag is hooked up to a clang command line option
-msahf/-mno-sahf specifically to give control of the 64-bit mode
behavior.

In the backend X86Subtarget constructor we were explicitly forcing
+sahf into the feature flag string if we were not compiling for
64-bit mode. This was intended to make the predicates always allow
the instructions outside of 64-bit mode. Unfortunately, the way
it was placed into the string allowed -mno-sahf from clang to disable
SAHF instructions in 32-bit mode. This causes an assertion to fire
if you compile a floating point comparison with something like
"-march=pentium -mno-sahf" as our floating point comparison
handling on CPUs that don't support FCOMI/FUCOMI instructions
requires SAHF.

To fix this, this commit restricts the feature flag to only apply to
64-bit mode by ignoring the flag outside 64-bit mode in
X86Subtarget::hasLAHFSAHF(). This way we don't need to mess with
the feature string at all.

Added: 
    

Modified: 
    llvm/lib/Target/X86/X86.td
    llvm/lib/Target/X86/X86Subtarget.cpp
    llvm/lib/Target/X86/X86Subtarget.h
    llvm/test/CodeGen/X86/setuge.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/X86/X86.td b/llvm/lib/Target/X86/X86.td
index 8ca6dac03675..d3225146d5bc 100644
--- a/llvm/lib/Target/X86/X86.td
+++ b/llvm/lib/Target/X86/X86.td
@@ -234,8 +234,8 @@ def FeaturePRFCHW  : SubtargetFeature<"prfchw", "HasPRFCHW", "true",
                                       "Support PRFCHW instructions">;
 def FeatureRDSEED  : SubtargetFeature<"rdseed", "HasRDSEED", "true",
                                       "Support RDSEED instruction">;
-def FeatureLAHFSAHF : SubtargetFeature<"sahf", "HasLAHFSAHF", "true",
-                                       "Support LAHF and SAHF instructions">;
+def FeatureLAHFSAHF : SubtargetFeature<"sahf", "HasLAHFSAHF64", "true",
+                           "Support LAHF and SAHF instructions in 64-bit mode">;
 def FeatureMWAITX  : SubtargetFeature<"mwaitx", "HasMWAITX", "true",
                                       "Enable MONITORX/MWAITX timer functionality">;
 def FeatureCLZERO  : SubtargetFeature<"clzero", "HasCLZERO", "true",

diff  --git a/llvm/lib/Target/X86/X86Subtarget.cpp b/llvm/lib/Target/X86/X86Subtarget.cpp
index 8a2d1130914e..43888d9adf1b 100644
--- a/llvm/lib/Target/X86/X86Subtarget.cpp
+++ b/llvm/lib/Target/X86/X86Subtarget.cpp
@@ -249,14 +249,6 @@ void X86Subtarget::initSubtargetFeatures(StringRef CPU, StringRef FS) {
     }
   }
 
-  // LAHF/SAHF are always supported in non-64-bit mode.
-  if (!In64BitMode) {
-    if (!FullFS.empty())
-      FullFS = "+sahf," + FullFS;
-    else
-      FullFS = "+sahf";
-  }
-
   // Parse features string and set the CPU.
   ParseSubtargetFeatures(CPU, FullFS);
 

diff  --git a/llvm/lib/Target/X86/X86Subtarget.h b/llvm/lib/Target/X86/X86Subtarget.h
index de45d357e3c2..0479e94825b0 100644
--- a/llvm/lib/Target/X86/X86Subtarget.h
+++ b/llvm/lib/Target/X86/X86Subtarget.h
@@ -191,8 +191,8 @@ class X86Subtarget final : public X86GenSubtargetInfo {
   /// Processor has RDSEED instructions.
   bool HasRDSEED = false;
 
-  /// Processor has LAHF/SAHF instructions.
-  bool HasLAHFSAHF = false;
+  /// Processor has LAHF/SAHF instructions in 64-bit mode.
+  bool HasLAHFSAHF64 = false;
 
   /// Processor has MONITORX/MWAITX instructions.
   bool HasMWAITX = false;
@@ -671,7 +671,7 @@ class X86Subtarget final : public X86GenSubtargetInfo {
     return hasSSE1() || (hasPRFCHW() && !has3DNow()) || hasPREFETCHWT1();
   }
   bool hasRDSEED() const { return HasRDSEED; }
-  bool hasLAHFSAHF() const { return HasLAHFSAHF; }
+  bool hasLAHFSAHF() const { return HasLAHFSAHF64 || !is64Bit(); }
   bool hasMWAITX() const { return HasMWAITX; }
   bool hasCLZERO() const { return HasCLZERO; }
   bool hasCLDEMOTE() const { return HasCLDEMOTE; }

diff  --git a/llvm/test/CodeGen/X86/setuge.ll b/llvm/test/CodeGen/X86/setuge.ll
index ea5562dbd510..3e0d6dedfd58 100644
--- a/llvm/test/CodeGen/X86/setuge.ll
+++ b/llvm/test/CodeGen/X86/setuge.ll
@@ -1,5 +1,7 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc < %s -mtriple=i686-- | FileCheck %s
+; Sanity check that we ignore -sahf in 32-bit mode rather than asserting.
+; RUN: llc < %s -mtriple=i686-- -mattr=-sahf | FileCheck %s
 
 declare i1 @llvm.isunordered.f32(float, float)