[PATCH] D31196: [InstCombine] fold insertelement of scalar constant into vector of constants

[Eli Friedman via Phabricator via llvm-commits]

efriedma added a comment.

Would it make sense for this transform to look through more than one insertelement?  Alternatively, we could perform this transform unconditionally: transform (insertelement (insertelement, X, Y, IdxC1), C, IdxC2) -> (insertelement (insertelement X, C, IdxC2), Y, IdxC1), ignoring the actual value of X.



================
Comment at: lib/Transforms/InstCombine/InstCombineVectorOps.cpp:745
+    }
+    if (match(IEI->getOperand(0), m_Constant(Val[1])) &&
+        InsertIdx[0] != InsertIdx[1]) {
----------------
Reusing Val[1] like this is confusing; can you use a separate variable?


================
Comment at: lib/Transforms/InstCombine/InstCombineVectorOps.cpp:758
+          // Copy over the existing values.
+          NewVectorC[I] = Val[1]->getAggregateElement(I);
+      }
----------------
getAggregateElement can return null.


https://reviews.llvm.org/D31196