[X86][clang] Enable floating-point type for -mno-x87 option on 32-bits

[pengfei]

Summary

We should match GCC's behavior which allows floating-point type for -mno-x87 option on 32-bits. https://godbolt.org/z/KrbhfWc9o

The previous block issues have partially been fixed by D112143.

[asavonic]

clang/lib/Basic/Targets/X86.cpp

385–388

I see that D112143 changed the ABI so that FP return values do not use x87 registers on i386. Therefore HasFPReturn flag can be removed.

However, operations with long double (x87 80-bit) should still be unsupported on both targets, because IIRC there is no SSE equivalent for them. GCC compiles them as soft-fp when -mno-x87 is set, but I haven't found 80-bit soft-fp implementation in LLVM.

long double baz(long double a, long double b) {
    return a + b;
}

baz:
   [...]
   call  __addxf3

For some reason GCC only does this for for i386 target, for x86_64 it just emits the diagnostic about disabled x87.

[pengfei]

clang/lib/Basic/Targets/X86.cpp

385–388

Thanks for looking at this patch.
I don't think we need to exclude f80 particularly. IIUC, backend tries all possible ways to lower a given operation. Lowering to library is always the last choice. So the behavior is not confined to soft-fp.
It's true LLVM has problems with f80 lowering without x87. I commented it in D112143 and hope D100091 will fix them. We don't need to bother to change it again in future.

For some reason GCC only does this for for i386 target, for x86_64 it just emits the diagnostic about disabled x87.

I think the root reason is the difference in ABI. 32-bits ABI allows passing and returning f80 without x87 registers while 64-bits doesn't. So we have to and only need to disable it for x86_64.

[asavonic]

clang/lib/Basic/Targets/X86.cpp

385–388

I don't think we need to exclude f80 particularly. IIUC, backend tries all possible ways to lower a given operation. Lowering to library is always the last choice. So the behavior is not confined to soft-fp.
It's true LLVM has problems with f80 lowering without x87. I commented it in D112143 and hope D100091 will fix them. We don't need to bother to change it again in future.

Right, but can LLVM lower any x87 80-bit fp operations other than return values?
If it cannot, then I think a source level diagnostic is a good thing to have. Otherwise the only handling we have is the codegen crash with "x87 register return with x87 disabled" and no source-level context.

[pengfei]

Emit diagnostic for long double for i386 target too.

[pengfei]

clang/lib/Basic/Targets/X86.cpp

385–388

No. I checked we are able to lower with changes like below. But it requires enabling all operations with full test. So emitting diagnostic seems good for now.

--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -729,6 +729,8 @@ X86TargetLowering::X86TargetLowering(const X86TargetMachine &TM,
     // FIXME: When the target is 64-bit, STRICT_FP_ROUND will be overwritten
     // as Custom.
     setOperationAction(ISD::STRICT_FP_ROUND, MVT::f80, Legal);
+  } else {
+    setOperationAction(ISD::FADD,        MVT::f80, LibCall);
   }

   // f128 uses xmm registers, but most operations require libcalls.


--- a/llvm/lib/Target/X86/X86InstrFormats.td
+++ b/llvm/lib/Target/X86/X86InstrFormats.td
@@ -472,6 +472,7 @@ class Ii32PCRel<bits<8> o, Format f, dag outs, dag ins, string asm,
 class FPI<bits<8> o, Format F, dag outs, dag ins, string asm>
   : I<o, F, outs, ins, asm, []> {
   let Defs = [FPSW];
+  let Predicates = [HasX87];
 }

 // FpI_ - Floating Point Pseudo Instruction template. Not Predicated.
@@ -479,6 +480,7 @@ class FpI_<dag outs, dag ins, FPFormat fp, list<dag> pattern>
   : PseudoI<outs, ins, pattern> {
   let FPForm = fp;
   let Defs = [FPSW];
+  let Predicates = [HasX87];
 }

 // Templates for instructions that use a 16- or 32-bit segmented address as


--- a/llvm/lib/Target/X86/X86InstrInfo.td
+++ b/llvm/lib/Target/X86/X86InstrInfo.td
@@ -929,6 +929,7 @@ def HasAES       : Predicate<"Subtarget->hasAES()">;
 def HasVAES      : Predicate<"Subtarget->hasVAES()">;
 def NoVLX_Or_NoVAES : Predicate<"!Subtarget->hasVLX() || !Subtarget->hasVAES()">;
 def HasFXSR      : Predicate<"Subtarget->hasFXSR()">;
+def HasX87       : Predicate<"Subtarget->hasX87()">;
 def HasXSAVE     : Predicate<"Subtarget->hasXSAVE()">;
 def HasXSAVEOPT  : Predicate<"Subtarget->hasXSAVEOPT()">;
 def HasXSAVEC    : Predicate<"Subtarget->hasXSAVEC()">;

[asavonic]

LGTM. We can also remove all code related to HasFPReturn, it is no longer needed.

[nickdesaulniers]

Thanks for the patch!

[pengfei]

In D114162#3157604, @asavonic wrote:

LGTM. We can also remove all code related to HasFPReturn, it is no longer needed.

I think we still need this flag, maybe the error message should be changed to "SSE register return with SSE disabled"? https://godbolt.org/z/KcGf751GE
I can do it as a follow up.

clang/lib/Basic/Targets/X86.cpp

if ((FPMath == FP_SSE && SSELevel < SSE1) ||
     Diags.Report(diag::err_target_unsupported_fpmath)
         << (FPMath == FP_SSE ? "sse" : "387");
     return false;
   }
 
   SimdDefaultAlign =
       hasFeature("avx512f") ? 512 : hasFeature("avx") ? 256 : 128;
 
-  if (!HasX87) {
-    if (LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
-      HasLongDouble = false;
-    if (getTriple().getArch() == llvm::Triple::x86)
-      HasFPReturn = false;
-  }
+  // FIXME: We should allow long double type on 32-bits to match with GCC.
+  // This requires backend to be able to lower f80 without x87 first.
+  if (!HasX87 && LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
 
   return true;
 }
 
 /// X86TargetInfo::getTargetDefines - Return the set of the X86-specific macro
 /// definitions for this particular subtarget.
 void X86TargetInfo::getTargetDefines(const LangOptions &Opts,
                                      MacroBuilder &Builder) const {

clang/test/Sema/x86-no-x87.cpp

-// RUN: %clang_cc1 -fsyntax-only -verify %s -triple i686-linux-gnu -target-feature -x87 -DRET_ERROR
+// RUN: %clang_cc1 -fsyntax-only -verify %s -triple i686-linux-gnu -target-feature -x87
 // RUN: %clang_cc1 -fsyntax-only -verify %s -triple i686-linux-gnu -DNOERROR
 
 #ifdef NOERROR
 // expected-no-diagnostics
 #endif
 
 typedef long double long_double;
 
#endif
   st.ld = d;
 }
 
 void assign5() {
   // unused variable declaration is fine
   long_double ld = 0.42;
 }
 
-#ifndef NOERROR
-// expected-note@+3{{'d_ret1' defined here}}
-// expected-error@+2{{'d_ret1' requires  'double' return type support, but target 'i686-unknown-linux-gnu' does not support it}}
-#endif
 double d_ret1(float x) {
   return 0.0;
 }
 
-#ifndef NOERROR
-// expected-note@+2{{'d_ret2' defined here}}
-#endif
 double d_ret2(float x);
 
 int d_ret3(float x) {
-#ifndef NOERROR
-  // expected-error@+2{{'d_ret2' requires  'double' return type support, but target 'i686-unknown-linux-gnu' does not support it}}
-#endif
   return (int)d_ret2(x);
 }
 
-#ifndef NOERROR
-// expected-note@+3{{'f_ret1' defined here}}
-// expected-error@+2{{'f_ret1' requires  'float' return type support, but target 'i686-unknown-linux-gnu' does not support it}}
-#endif
 float f_ret1(float x) {
   return 0.0f;
 }
 
-#ifndef NOERROR
-// expected-note@+2{{'f_ret2' defined here}}
-#endif
 float f_ret2(float x);
 
 int f_ret3(float x) {
-#ifndef NOERROR
-  // expected-error@+2{{'f_ret2' requires  'float' return type support, but target 'i686-unknown-linux-gnu' does not support it}}
-#endif
   return (int)f_ret2(x);
 }