[PATCH] D141763: [X86] Remove unnecessary bitcasting in INT_TO_FP

icedrocket via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat Jan 14 07:08:55 PST 2023


icedrocket created this revision.
Herald added subscribers: pengfei, hiraditya.
Herald added a project: All.
icedrocket requested review of this revision.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

This seems to have been added to prevent two 32-bit stores in older versions. But the latest version doesn't seem to use two 32-bit stores for casting. This increases the stack size and causes unnecessary bitcasting. Do we still need this?


https://reviews.llvm.org/D141763

Files:
  llvm/lib/Target/X86/X86ISelLowering.cpp


Index: llvm/lib/Target/X86/X86ISelLowering.cpp
===================================================================
--- llvm/lib/Target/X86/X86ISelLowering.cpp
+++ llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -21368,13 +21368,6 @@
   if (VT == MVT::f128 || !Subtarget.hasX87())
     return SDValue();
 
-  SDValue ValueToStore = Src;
-  if (SrcVT == MVT::i64 && Subtarget.hasSSE2() && !Subtarget.is64Bit())
-    // Bitcasting to f64 here allows us to do a single 64-bit store from
-    // an SSE register, avoiding the store forwarding penalty that would come
-    // with two 32-bit stores.
-    ValueToStore = DAG.getBitcast(MVT::f64, ValueToStore);
-
   unsigned Size = SrcVT.getStoreSize();
   Align Alignment(Size);
   MachineFunction &MF = DAG.getMachineFunction();
@@ -21383,7 +21376,7 @@
   MachinePointerInfo MPI =
       MachinePointerInfo::getFixedStack(DAG.getMachineFunction(), SSFI);
   SDValue StackSlot = DAG.getFrameIndex(SSFI, PtrVT);
-  Chain = DAG.getStore(Chain, dl, ValueToStore, StackSlot, MPI, Alignment);
+  Chain = DAG.getStore(Chain, dl, Src, StackSlot, MPI, Alignment);
   std::pair<SDValue, SDValue> Tmp =
       BuildFILD(VT, SrcVT, dl, Chain, StackSlot, MPI, Alignment, DAG);
 
@@ -21880,15 +21873,8 @@
   }
 
   assert(SrcVT == MVT::i64 && "Unexpected type in UINT_TO_FP");
-  SDValue ValueToStore = Src;
-  if (isScalarFPTypeInSSEReg(Op.getValueType()) && !Subtarget.is64Bit()) {
-    // Bitcasting to f64 here allows us to do a single 64-bit store from
-    // an SSE register, avoiding the store forwarding penalty that would come
-    // with two 32-bit stores.
-    ValueToStore = DAG.getBitcast(MVT::f64, ValueToStore);
-  }
-  SDValue Store =
-      DAG.getStore(Chain, dl, ValueToStore, StackSlot, MPI, SlotAlign);
+
+  SDValue Store = DAG.getStore(Chain, dl, Src, StackSlot, MPI, SlotAlign);
   // For i64 source, we need to add the appropriate power of 2 if the input
   // was negative. We must be careful to do the computation in x87 extended
   // precision, not in SSE.
@@ -21899,7 +21885,6 @@
                               SlotAlign, MachineMemOperand::MOLoad);
   Chain = Fild.getValue(1);
 
-
   // Check whether the sign bit is set.
   SDValue SignSet = DAG.getSetCC(
       dl, getSetCCResultType(DAG.getDataLayout(), *DAG.getContext(), MVT::i64),


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D141763.489243.patch
Type: text/x-patch
Size: 2297 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20230114/75d58466/attachment.bin>


More information about the llvm-commits mailing list