[PATCH] D15240: X86: Don't emit SAHF/LAHF for 64-bit targets unless explicitly supported

JF Bastien via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 4 11:54:47 PST 2015


jfb added a comment.

I haven't looked through the list of CPU support, or tests yet.


================
Comment at: lib/Target/X86/X86.td:186
@@ -185,1 +185,3 @@
+def FeatureSAHF : SubtargetFeature<"sahf", "HasSAHF", "true",
+                                   "Support SAHF and LAHF instructions">;
 def FeatureMPX     : SubtargetFeature<"mpx", "HasMPX", "true",
----------------
I like my naming of `HasLAHFSAHF` since it's more comprehensive.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:13935
@@ -13933,1 +13934,3 @@
+  // Some 64-bit targets lack SAHF support, but they do support FCOMI.
+  assert(Subtarget->hasSAHF() && "Target doesn't support SAHF or FCOMI?");
   return DAG.getNode(X86ISD::SAHF, dl, MVT::i32, TruncSrl);
----------------
This should be fixed before committing?

================
Comment at: lib/Target/X86/X86InstrInfo.cpp:4468
@@ -4436,2 +4467,3 @@
+		}
   }
 
----------------
I like my format for `lib/Target/X86/X86InstrInfo.cpp` in D15239 better ☺

================
Comment at: lib/Target/X86/X86Subtarget.cpp:200
@@ +199,3 @@
+  }
+#endif
+
----------------
What name does GCC use?


http://reviews.llvm.org/D15240





More information about the llvm-commits mailing list