[PATCH] D13979: Introduction of FeatureX87

[Eric Christopher via llvm-commits]

echristo added a subscriber: echristo.
echristo added a comment.

You've done this on an old version of the sources, you'll need to rebase.

One other reply inline.

Thanks!

-eric


================
Comment at: lib/Target/X86/X86Subtarget.h:406
@@ -401,3 +405,3 @@
   bool isSLM() const { return X86ProcFamily == IntelSLM; }
-  bool useSoftFloat() const { return UseSoftFloat; }
+  bool useSoftFloat() const { return UseSoftFloat || (!hasX87() && !hasSSE2()); }
 
----------------
aturetsk wrote:
> bruno wrote:
> > This looks odd since we do support f32 (not f64) with SSE1. See X86ISelLowering.cpp:553
> > 
> > } else if (!Subtarget->useSoftFloat() && X86ScalarSSEf32) {
> >     // Use SSE for f32, x87 for f64.
> >     // Set up the FP register classes.
> >     addRegisterClass(MVT::f32, &X86::FR32RegClass);
> >     addRegisterClass(MVT::f64, &X86::RFP64RegClass);
> > ...
> > 
> > Since not having SSE at all fallbacks to x87, why not only check for "UseSoftFloat ||  !hasX87())" ? Anyway, I think those should come in a separate patch with appropriate feature testcases. 
> My logic came from the hypothetical situation when we have SSE/SSE2, but not X87. If we have SSE there is no instructions to handle f64, thus without X87 we should use soft floats. SSE2 can handle f64 so there is no need to do that.
I agree with Bruno here. It also might be a good idea to not add this conditional at all and just use the legalizer. Thoughts?


http://reviews.llvm.org/D13979