[PATCH] D13979: Introduction of FeatureX87

[Andrey Turetskiy via llvm-commits]

aturetsk added inline comments.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:527
@@ -525,3 +526,3 @@
 
-  if (!Subtarget.useSoftFloat() && X86ScalarSSEf64) {
+  if (UseX87 && X86ScalarSSEf64) {
     // f32 and f64 use SSE.
----------------
bruno wrote:
> aturetsk wrote:
> > I get X87 load and store instructions in x87.ll if I don't check hasX87 here. I think changing that would require significant efforts. Since we don't have a CPU which has -x87 but +sse2, I just left the check here.
> This looks odd, do you know why it happens? in which specific target feature combination?
This happens with -x87,+sse2 in 32 bit mode (64 bit mode seems to be OK).
The instruction to blame is "sitofp i64 %l to float". Actually I was able to get rid of fild instruction and have a soft float call by adding the condition at line 213:
```
if(Subtarget.hasX87() || Subtarget.is64Bit())
    setOperationAction(ISD::SINT_TO_FP     , MVT::i64  , Custom);
```
However I still get the wrong fstp instruction:
```
        calll   __floatdisf
        fstps   20(%esp)
        movss   20(%esp), %xmm0         # xmm0 = mem[0],zero,zero,zero
```

But what worries me the most is that there may be more such non-obvious cases, even in the current version of the patch having the test passed.
Probably we should use more straight-forward and easier way (until we have a real case where the best code for -x87,+sse/sse2 is required) - to have a variable "UseX87 = !useSoftFloat() && hasX87()" and replace useSoftFloat() with !useX87 absolutely everywhere (similar to what was done initially, but keep useSoftFloat() unchanged through the use of a new variable).
This way would guarantee that the compiler generates correct code with disabled x87.


http://reviews.llvm.org/D13979