[PATCH] D137688: [CodeGen] Refactor visitSCALAR_TO_VECTOR. NFC.

[Sanjay Patel via Phabricator via llvm-commits]

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

Less readable to me, but shorter, so I'll say LGTM - see inline for some possible minor improvements.

Note that there is potentially a real cost to writing code like this:
https://github.com/llvm/llvm-project/issues/58982



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:23529
 
-    // s2v (bo (extelt V, Idx), C) --> shuffle (bo V, C'), {Idx, -1, -1...}
-    if (auto *C = dyn_cast<ConstantSDNode>(Scalar.getOperand(1))) {
-      if (getShuffleMaskForExtElt(Scalar.getOperand(0))) {
-        SDLoc DL(N);
-        SDValue V = Scalar.getOperand(0).getOperand(0);
-        SDValue VecC = DAG.getConstant(C->getAPIntValue(), DL, VT);
-        SDValue VecBO = DAG.getNode(Opcode, DL, VT, V, VecC);
-        return DAG.getVectorShuffle(VT, DL, VecBO, DAG.getUNDEF(VT), ShufMask);
-      }
-    }
-    // s2v (bo C, (extelt V, Idx)) --> shuffle (bo C', V), {Idx, -1, -1...}
-    if (auto *C = dyn_cast<ConstantSDNode>(Scalar.getOperand(0))) {
-      if (getShuffleMaskForExtElt(Scalar.getOperand(1))) {
-        SDLoc DL(N);
-        SDValue V = Scalar.getOperand(1).getOperand(0);
-        SDValue VecC = DAG.getConstant(C->getAPIntValue(), DL, VT);
-        SDValue VecBO = DAG.getNode(Opcode, DL, VT, VecC, V);
-        return DAG.getVectorShuffle(VT, DL, VecBO, DAG.getUNDEF(VT), ShufMask);
+    for (int i = 0; i != 2; ++i) {
+      // s2v (bo (extelt V, Idx), C) --> shuffle (bo V, C'), {Idx, -1, -1...}
----------------
I find it easier to notice pseudo-loops like this if we write it this way:
    for (int i : {0, 1}) {



================
Comment at: llvm/lib/CodeGen/SelectionDAG/DAGCombiner.cpp:23532-23536
+      if (auto *C = dyn_cast<ConstantSDNode>(Scalar.getOperand(1 - i))) {
+        SDValue EE = Scalar.getOperand(i);
+        if (EE.getOpcode() == ISD::EXTRACT_VECTOR_ELT &&
+            EE.getOperand(0).getValueType() == VT &&
+            isa<ConstantSDNode>(EE.getOperand(1))) {
----------------
Reduce levels of indents:
      SDValue EE = Scalar.getOperand(i);
      auto *C = dyn_cast<ConstantSDNode>(Scalar.getOperand(i ? 0 : 1));
      if (C && EE.getOpcode() == ...)

Also,  a "i ? 0 : 1" pattern seems more obvious to me than "1 - i".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137688