Resolve TODO: Use TokenFactor for inline memset

[AtariDreams]

We can rewrite this as a TokenFactor like memcpy is.

[llvmbot]

@llvm/pr-subscribers-backend-x86

Author: AtariDreams (AtariDreams)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/87002.diff

1 Files Affected:

• (modified) llvm/lib/Target/X86/X86SelectionDAGInfo.cpp (+23-20)

diff --git a/llvm/lib/Target/X86/X86SelectionDAGInfo.cpp b/llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
index 7c630a2b0da080..0f46110f22dc93 100644
--- a/llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
+++ b/llvm/lib/Target/X86/X86SelectionDAGInfo.cpp
@@ -67,7 +67,7 @@ SDValue X86SelectionDAGInfo::EmitTargetCodeForMemset(
   // The libc version is likely to be faster for these cases. It can use the
   // address value and run time information about the CPU.
   if (Alignment < Align(4) || !ConstantSize ||
-      ConstantSize->getZExtValue() > Subtarget.getMaxInlineSizeThreshold()) 
+      ConstantSize->getZExtValue() > Subtarget.getMaxInlineSizeThreshold())
     return SDValue();
 
   uint64_t SizeVal = ConstantSize->getZExtValue();
@@ -128,26 +128,29 @@ SDValue X86SelectionDAGInfo::EmitTargetCodeForMemset(
   InGlue = Chain.getValue(1);
 
   SDVTList Tys = DAG.getVTList(MVT::Other, MVT::Glue);
-  SDValue Ops[] = { Chain, DAG.getValueType(AVT), InGlue };
-  Chain = DAG.getNode(X86ISD::REP_STOS, dl, Tys, Ops);
-
-  if (BytesLeft) {
-    // Handle the last 1 - 7 bytes.
-    unsigned Offset = SizeVal - BytesLeft;
-    EVT AddrVT = Dst.getValueType();
-    EVT SizeVT = Size.getValueType();
-
-    Chain =
-        DAG.getMemset(Chain, dl,
-                      DAG.getNode(ISD::ADD, dl, AddrVT, Dst,
-                                  DAG.getConstant(Offset, dl, AddrVT)),
-                      Val, DAG.getConstant(BytesLeft, dl, SizeVT), Alignment,
-                      isVolatile, AlwaysInline,
-                      /* isTailCall */ false, DstPtrInfo.getWithOffset(Offset));
-  }
+  SDValue Ops[] = {Chain, DAG.getValueType(AVT), InGlue};
+  SDValue RepStos = DAG.getNode(X86ISD::REP_STOS, dl, Tys, Ops);
+
+  /// RepStos can process the whole length.
+  if (BytesLeft == 0)
+    return RepStos;
 
-  // TODO: Use a Tokenfactor, as in memcpy, instead of a single chain.
-  return Chain;
+  // Handle the last 1 - 7 bytes.
+  SmallVector<SDValue, 4> Results;
+  Results.push_back(RepStos);
+  unsigned Offset = SizeVal - BytesLeft;
+  EVT AddrVT = Dst.getValueType();
+  EVT SizeVT = Size.getValueType();
+
+  Results.push_back(
+      DAG.getMemset(Chain, dl,
+                    DAG.getNode(ISD::ADD, dl, AddrVT, Dst,
+                                DAG.getConstant(Offset, dl, AddrVT)),
+                    Val, DAG.getConstant(BytesLeft, dl, SizeVT), Alignment,
+                    isVolatile, AlwaysInline,
+                    /* isTailCall */ false, DstPtrInfo.getWithOffset(Offset)));
+
+  return DAG.getNode(ISD::TokenFactor, dl, MVT::Other, Results);
 }
 
 /// Emit a single REP MOVS{B,W,D,Q} instruction.

[XiaodongLoong]

Could you please add a .ll test for the PR?

[AtariDreams]

Could you please add a .ll test for the PR?

It's a refactor.

[phoebewang]

I don't see problem to refactor to TokenFactor, but it's not a NFC. Maybe it's OK if it's not easy to create a test case, but should remove [NFC] from title.

[phoebewang]

LGTM.

[RKSimon]

@AtariDreams Please can you update the patch subject to be a more clear description of where/what the patch is doing

[AtariDreams]

@AtariDreams Please can you update the patch subject to be a more clear description of where/what the patch is doing

Done!