[PATCH] D31651: Fix in SelectionDAG::getNode() to not produce illegal BUILD_VECTOR operands

[Jonas Paulsson via Phabricator via llvm-commits]

jonpa added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:4143
+      llvm::EVT LegalScalarVT = (TLI->isTypeLegal(VT.getScalarType()) ?
+         VT.getScalarType() : N1.getOperand(0).getValueType().getScalarType());
+
----------------
niravd wrote:
> It doesn't seems like we should have to consider if the type is legal. VT should never have been used. 
> 
> We should be able to use Op.getValueType().getScalarType() wherever LegalScalarVT is used. 
Changed so the BUILD_VECTOR operand type is unchanged in the new BUILD_VECTOR. I thought first there might be a point in having a legal smaller operand type, but at second thought there seems to be no point as BUILD_VECTOR will truncate and the smaller constant operands should get put into the smaller vector still.

Also removed the call to zextOrTrunc() since we keep the same VT for Op.



================
Comment at: lib/CodeGen/SelectionDAG/SelectionDAG.cpp:4148
         SDValue Op = N1.getOperand(i);
-        if (Op.isUndef()) {
-          Ops.push_back(getUNDEF(VT.getScalarType()));
-          continue;
-        }
-        if (ConstantSDNode *C = dyn_cast<ConstantSDNode>(Op)) {
+        if (Op.isUndef())
+          Ops.push_back(getUNDEF(LegalScalarVT));
----------------
niravd wrote:
> This removal of the redundant check seems reasonable and should probably be done as a separate NFC cleanup. 
OK, will do a separate NFC commit for the redundant check.


https://reviews.llvm.org/D31651