[PATCH] D68857: [X86] Add strict fp support for operations of X87 instructions

[LiuChen via Phabricator via llvm-commits]

LiuChen3 marked an inline comment as done.
LiuChen3 added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:601
+
+    // Handle constrained floating-point operations of scalar.
+    for (auto VT : { MVT::f32, MVT::f64, MVT::f80 }) {
----------------
craig.topper wrote:
> Doesn't this stop working if sse1 is enabled? By we still need x87 for f64/f80.
Yes . This can only be enabled when disable sse.  I'll find which will still be needed when SSE enabled.  I think most of them are related to f80. 


================
Comment at: llvm/lib/Target/X86/X86InstrFPStack.td:372
 let SchedRW = [WriteMicrocoded] in {
-defm SIN : FPUnary<fsin, MRM_FE, "fsin">;
-defm COS : FPUnary<fcos, MRM_FF, "fcos">;
+defm SIN : FPUnary<any_fsin, MRM_FE, "fsin">;
+defm COS : FPUnary<any_fcos, MRM_FF, "fcos">;
----------------
craig.topper wrote:
> SIN/COS are not mentioned in the X86ISelLowering code you added.
X86 uses library to calculate SIN/COS. X87 set SIN/COS as Expand. So I leave strict_fsin/strict_fcos as "expand" as default.


================
Comment at: llvm/test/CodeGen/X86/x87-fp-strict-sub.ll:84
+
+!0 = !{!1, !1, i64 0}
+!1 = !{!"float", !2, i64 0}
----------------
craig.topper wrote:
> Is all this metadata needed?
It's not necessay. I'll delete them.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D68857/new/

https://reviews.llvm.org/D68857