[PATCH] D109989: [X86] Improve i8 all-ones element insertion in pre-SSE4.1

[Craig Topper via Phabricator via llvm-commits]

craig.topper added a comment.

In D109989#3007196 <https://reviews.llvm.org/D109989#3007196>, @RKSimon wrote:

> @craig.topper Can you recall if there was any reason why we don't already support pre-SSE41 v16i8 lowering?

I don't recall anything.



================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:19315
+      SDValue OnesCst = DAG.getAllOnesConstant(dl, VT.getScalarType());
+      SDValue &KeepElt = IsZeroElt ? OnesCst : ZeroCst;
+      SDValue &OverrideElt = !IsZeroElt ? OnesCst : ZeroCst;
----------------
Why references?


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:19317
+      SDValue &OverrideElt = !IsZeroElt ? OnesCst : ZeroCst;
+      for (unsigned i = 0; i != NumElts; ++i)
+        CstVectorElts.push_back(i == IdxVal ? OverrideElt : KeepElt);
----------------
Can't you just fill the vector with the Keep value using the SmallVector constructor, and then just replace element i with the OverrideElt after its constructed? No loop required.


================
Comment at: llvm/lib/Target/X86/X86ISelLowering.cpp:19320
+      SDValue CstVector = DAG.getBuildVector(VT, dl, CstVectorElts);
+      return DAG.getNode(IsZeroElt ? ISD::AND : ISD::OR, dl, VT, N0, CstVector);
+    }
----------------
Is the AND case not tested? All the test changes have Or.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109989