[PATCH] D13979: Introduction of FeatureX87

[Bruno Cardoso Lopes via llvm-commits]

bruno added inline comments.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:526
@@ -525,3 +525,3 @@
 
-  if (!Subtarget->useSoftFloat() && X86ScalarSSEf64) {
+  if (!Subtarget->useSoftFloat() && Subtarget->hasX87() && X86ScalarSSEf64) {
     // f32 and f64 use SSE.
----------------
Why check Subtarget->hasX87() here? We're using SSE for f32 and f64 anyways

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:560
@@ -559,2 +559,3 @@
     addLegalFPImmediate(APFloat(+0.0f)); // xorps
-  } else if (!Subtarget->useSoftFloat() && X86ScalarSSEf32) {
+  } else if (!Subtarget->useSoftFloat() && Subtarget->hasX87() &&
+             X86ScalarSSEf32) {
----------------
Factor out "!Subtarget->useSoftFloat() && Subtarget->hasX87()" with

bool UseX87 = !Subtarget->useSoftFloat() && Subtarget->hasX87();

and use that throughout the checks. 

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:596
@@ -594,3 +595,3 @@
     }
-  } else if (!Subtarget->useSoftFloat()) {
+  } else if (!Subtarget->useSoftFloat() && Subtarget->hasX87()) {
     // f32 and f64 in x87.
----------------
UseX87 here

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:630
@@ -628,3 +629,3 @@
   // Long double always uses X87, except f128 in MMX.
-  if (!Subtarget->useSoftFloat()) {
+  if (!Subtarget->useSoftFloat() && Subtarget->hasX87()) {
     if (Subtarget->is64Bit() && Subtarget->hasMMX()) {
----------------
UseX87 here

================
Comment at: test/CodeGen/X86/x87.ll:7
@@ +6,3 @@
+; RUN: llc < %s -march=x86-64 -mattr=-x87,+sse | FileCheck %s -check-prefix=NOX87
+; RUN: llc < %s -march=x86 -mattr=-x87,+sse2 | FileCheck %s -check-prefix=NOX87
+; RUN: llc < %s -march=x86-64 -mattr=-x87,+sse2 | FileCheck %s -check-prefix=NOX87
----------------
This test needs improvements; you can make it tighter by removing the allocas and other unnecessary instructions. Please explicitly check for all the specific instructions you want to match.


http://reviews.llvm.org/D13979