[PATCH] D24683: [DAGCombine] Generalize build_vector -> vector_shuffle combine for more than 2 inputs
Michael Kuperstein via llvm-commits
llvm-commits at lists.llvm.org
Wed Sep 28 02:53:56 PDT 2016
mkuper added inline comments.
================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:13030
@@ +13029,3 @@
+ // All inputs must have the same element type as the output.
+ if (VT.getVectorElementType() !=
+ ExtractedFromVec.getValueType().getVectorElementType())
----------------
delena wrote:
> mkuper wrote:
> > delena wrote:
> > > I suppose you can put "assert" here.
> > I guess the comment is ambiguous - by "must" I didn't mean that we never get here if that doesn't happen, but that it's required for the combine. I'll change the comment.
> You can't create BUILD_VECTOR node of v8i32 and put i8 operands for it.
Yes, that's why I check that, and bail out if the types don't match. But this is the first place that actually checks this (I think?), so this has to be a failure return, not an assert.
================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:13038
@@ +13037,3 @@
+ unsigned Idx = std::distance(
+ VecIn.begin(), std::find(VecIn.begin(), VecIn.end(), ExtractedFromVec));
+ if (Idx == VecIn.size())
----------------
delena wrote:
> line alignment
Will fix, thanks.
================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:13045
@@ +13044,3 @@
+
+ // If we didn't find at least one input vector, bail out.
+ if (VecIn.size() < 2)
----------------
delena wrote:
> at least two?
VecIn[0] is reserved for the zero vector. So it's really at least one.
(We actually care about the case where it's exactly 1 - regardless of whether we're blending with 0 or not.)
================
Comment at: lib/CodeGen/SelectionDAG/DAGCombiner.cpp:13073
@@ +13072,3 @@
+ SDValue VecRight =
+ (LeftIdx + 1) < VecIn.size() ? VecIn[LeftIdx + 1] : SDValue();
+
----------------
delena wrote:
> line alignment
Will fix, thanks.
I was sure I ran clang-format over this...
https://reviews.llvm.org/D24683
More information about the llvm-commits
mailing list