[PATCH] D31651: Fix in SelectionDAG::getNode() to not produce illegal BUILD_VECTOR operands
Jonas Paulsson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Apr 5 01:05:32 PDT 2017
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
More information about the llvm-commits
mailing list