[PATCH] D17176: [CodeGen] Add getBuildVector and getSplatBuildVector helpers.

[Ahmed Bougacha via llvm-commits]

ab marked 2 inline comments as done.

================
Comment at: include/llvm/CodeGen/SelectionDAG.h:641
@@ +640,3 @@
+    return getNode(ISD::BUILD_VECTOR, DL, VT, Ops);
+  }
+
----------------
RKSimon wrote:
> ab wrote:
> > What do you think of moving the VerifySDNode checks here?
> I'm happy for them to stay where they are as long as:
> 
> (a) there isn't any way that the asserts can be avoided depending on the manner in which BUILD_VECTOR is created.
> 
> (b) the assert is directly in line in the call stack with whatever created the BUILD_VECTOR in the first place - we used to have cases where only a much later assert would fire regarding the inconsistent valuetypes leaving us with no idea what generated the faulty BUILD_VECTOR in the first place.
Yes, any new node goes through InsertNode and VerifySDNode; this is what a failed assert looks like:

9  llc                      0x0000000111bf8e22 VerifySDNode(llvm::SDNode*) + 1250
10 llc                      0x0000000111bf88f8 llvm::SelectionDAG::InsertNode(llvm::SDNode*) + 88
11 llc                      0x0000000111c05746 llvm::SelectionDAG::getNode(unsigned int, llvm::SDLoc, llvm::EVT, llvm::SDValue, llvm::SDValue, llvm::SDNodeFlags const*) + 17414
12 llc                      0x0000000111c07c75 llvm::SelectionDAG::getNode(unsigned int, llvm::SDLoc, llvm::EVT, llvm::ArrayRef<llvm::SDValue>, llvm::SDNodeFlags const*) + 661
13 llc                      0x0000000111c1da85 llvm::SelectionDAG::getBuildVector(llvm::EVT, llvm::SDLoc, llvm::ArrayRef<llvm::SDValue>) + 133


I can imagine one case where this is a problem though: if we ever add code to getNode that folds BUILD_VECTORs, that code will execute before the checks.

================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:3294-3299
@@ -3301,7 +3293,8 @@
           return Folded;
         SmallVector<SDValue, 4> Outputs;
+        llvm_unreachable("can't have vectors ops with scalar operands");
         // We may have a vector type but a scalar result. Create a splat.
         Outputs.resize(VT.getVectorNumElements(), Outputs.back());
         // Build a big vector out of the scalar elements we generated.
         return getNode(ISD::BUILD_VECTOR, SDLoc(), VT, Outputs);
       } else {
----------------
RKSimon wrote:
> ab wrote:
> > I think this block is unreachable: if two operands are constants, there's no way for this op to be vector.
> > 
> > If I didn't miss anything, I'll remove this separately.
> LCOV agrees in that no tests ever hit it. 
r266100


http://reviews.llvm.org/D17176