[PATCH] D14260: Optimize store of "bitcast" from vector to aggregate.

[Arch D. Robison via llvm-commits]

ArchDRobison added inline comments.

================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:857
@@ +856,3 @@
+///
+///     %x0 = extractelement <2 x double> %x, i32 0
+///     %i0 = insertvalue [2 x double] undef, double %x0, 0
----------------
hfinkel wrote:
> Please use %U<n> and %W<n> in this comment because you use the variables named U and W in the code (and that should make things clearer without necessitating longer variable names).
[I'm now back from sabbatical.]   Nice idea.  U and W actually refer to the same instruction.  I'll try using %U, %V, and %E since those correspond to U, V, and EI in the code.  I'll rename EI to E to make the correspondence cleaner. 

================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:877
@@ +876,3 @@
+    auto *CI = dyn_cast<ConstantInt>(EI->getIndexOperand());
+    if (!CI || IV->getNumIndices() != 1 || CI->getZExtValue() != *IV->idx_begin())
+      return nullptr;
----------------
hfinkel wrote:
> Do we need to check here that IV has only one index?
The check is necessary, since if IV has more than one index the code would have to do much trickier checking for equivalence to a vector.  Checking that the aggregate element type is a scalar type would have the same effect, though checking the number of indices seems like a more direct approach.

================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:899
@@ +898,3 @@
+    for (const Type *EltT : ST->elements()) {
+      if (EltT != UT->getElementType())
+        return nullptr;
----------------
hfinkel wrote:
> Do the types here need to agree, or just the type sizes?
At a minimum, sizes and alignments have to agree (otherwise inter-element padding might not match up exactly).  But I doubt the generalization is worth the extra complexity unless there are clear use cases that would benefit from it.


http://reviews.llvm.org/D14260