[PATCH] D81833: [X86][FPEnv] Lowering of {get,set,reset}_fpenv

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:30044
+                                   SelectionDAG &DAG) {
+  // Set x87 state.
+  Chain = DAG.getMemIntrinsicNode(X86ISD::FLDENV, DL, DAG.getVTList(MVT::Other),
----------------
Maybe check for `hasX87` too?


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:30048
+  // If target supports SSE, set MXCSR as well.
+  if (DAG.getSubtarget().hasFeature(X86::FeatureSSE1)) {
+    // Get pointer to the MXCSR location in memory.
----------------
`hasSSE1`?


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:30080
+
+  const auto &Subtarget = static_cast<const X86Subtarget &>(DAG.getSubtarget());
+  IntegerType *ItemTy = Type::getInt32Ty(*DAG.getContext());
----------------
Maybe pass `Subtarget` from caller instead?


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.h:837
+    // Store x87 FPU environment into memory.
+    FNSTENV,
+
----------------
Add a suffix `m`


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.h:840
+    // Load x87 FPU environment from memory.
+    FLDENV,
+
----------------
ditto.


================
Comment at: llvm/lib/Target/X86/X86InstrFPStack.td:428
 let Defs = [FPSW, FPCW], mayLoad = 1 in {
-def FLDENVm  : FPI<0xD9, MRM4m, (outs), (ins anymem:$src), "fldenv\t$src">;
+def FLDENVm  : I<0xD9, MRM4m, (outs), (ins anymem:$src), "fldenv\t$src",
+                 [(X86fpenv_set addr:$src)]>;
----------------
Need to add `Predicates = [HasX87]` if change to `I`.


================
Comment at: llvm/lib/Target/X86/X86InstrFPStack.td:434
 let Defs = [FPSW, FPCW], Uses = [FPSW, FPCW], mayStore = 1 in {
-def FSTENVm  : FPI<0xD9, MRM6m, (outs), (ins anymem:$dst), "fnstenv\t$dst">;
+def FSTENVm  : I<0xD9, MRM6m, (outs), (ins anymem:$dst), "fnstenv\t$dst",
+                 [(X86fpenv_get addr:$dst)]>;
----------------
ditto.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81833