[PATCH] D71968: [X86] Re-enable lowerUINT_TO_FP_vXi32 under fast-math by using an FSUB instead of an FADD.

Thu Jan 2 13:27:55 PST 2020

spatel accepted this revision.
spatel added a comment.
This revision is now accepted and ready to land.

LGTM - we could do what the existing code comment suggests, but it's more work, and I don't think it's worth the effort.

================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:19051-19053
-  // FIXME: If we ever have some kind of Machine FMF, this should be marked
-  // as non-fast and always be enabled. Why isn't SDAG FMF enough? Because
-  // there's also the MachineCombiner reassociations happening on Machine IR.
----------------
We actually do have FMF on MIR now - see D52006 and related patches.

================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:19099

-  // Create the vector constant for -(0x1.0p39f + 0x1.0p23f).
-  SDValue VecCstFAdd = DAG.getConstantFP(
-      APFloat(APFloat::IEEEsingle(), APInt(32, 0xD3000080)), DL, VecFloatVT);
+  // Create the vector constant for (0x1.0p39f + 0x1.0p23f).
+  SDValue VecCstFSub = DAG.getConstantFP(
----------------
Better leave a code comment here to explain that this is fragile and requires subtracting the positive constant rather than adding the negative constant.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71968