[llvm-commits] [llvm] r69952 - in /llvm/trunk: include/llvm/CodeGen/ include/llvm/Target/ lib/CodeGen/SelectionDAG/ lib/Target/CellSPU/ lib/Target/PowerPC/ lib/Target/X86/ test/CodeGen/X86/ utils/TableGen/
Duncan Sands
baldrick at free.fr
Fri Apr 24 02:18:32 PDT 2009
Hi Nate, thanks for doing this.
> +class ShuffleVectorSDNode : public SDNode {
> + SDUse Ops[2];
> + int *Mask;
> +protected:
> + friend class SelectionDAG;
> + ShuffleVectorSDNode(MVT VT, DebugLoc dl, SDValue N1, SDValue N2, int *M)
> + : SDNode(ISD::VECTOR_SHUFFLE, dl, getSDVTList(VT)), Mask(M) {
> + InitOperands(Ops, N1, N2);
> + }
> +public:
> +
> + const int * getMask() const { return Mask; }
It would be easy to go off the end of the mask when accessing mask
elements. How about having methods to access individual mask
elements, or turn Mask into a {int *, size} pair, which accessors
that check for overflow?
> - unsigned Idx = cast<ConstantSDNode>(InVec.getOperand(2).
> - getOperand(Elt))->getZExtValue();
> - unsigned NumElems = InVec.getOperand(2).getNumOperands();
> + int Idx = cast<ShuffleVectorSDNode>(InVec)->getMask()[Elt];
> + int NumElems = InVec.getValueType().getVectorNumElements();
What happens here if Idx < 0?
> SDValue DAGCombiner::visitVECTOR_SHUFFLE(SDNode *N) {
> - SDValue ShufMask = N->getOperand(2);
> - unsigned NumElts = ShufMask.getNumOperands();
> + return SDValue();
> +
> + MVT VT = N->getValueType(0);
> + unsigned NumElts = VT.getVectorNumElements();
Isn't this code unreachable since you always return SDValue()?
> @@ -762,12 +769,6 @@
> assert(N->getValueType(0).isVector() && "Wrong return type!");
> assert(N->getNumOperands() == N->getValueType(0).getVectorNumElements() &&
> "Wrong number of operands!");
> - MVT EltVT = N->getValueType(0).getVectorElementType();
> - for (SDNode::op_iterator I = N->op_begin(), E = N->op_end(); I != E; ++I)
> - assert((I->getValueType() == EltVT ||
> - (EltVT.isInteger() && I->getValueType().isInteger() &&
> - EltVT.bitsLE(I->getValueType()))) &&
> - "Wrong operand type!");
Why are you removing this assertion?
> + // Validate that all the indices past in in Mask are within the range of
past in in -> passed in
> + // elements input to the shuffle.
> + int NElts = VT.getVectorNumElements();
> + SmallVector<int, 8> MaskVec;
> + for (int i = 0; i != NElts; ++i) {
> + if (Mask[i] >= (NElts * 2)) {
> + assert(0 && "Index out of range");
> + return SDValue();
> + }
> + MaskVec.push_back(Mask[i]);
> + }
You should probably also check that any negative values are
actually equal to -1.
> + // Canonicalize all index into lhs, -> shuffle lhs, undef
> + // Canonicalize all index into rhs, -> shuffle rhs, undef
> + bool AllLHS = true, AllRHS = true;
> + bool N2Undef = N2.getOpcode() == ISD::UNDEF;
> + for (int i = 0; i != NElts; ++i) {
> + if (MaskVec[i] >= NElts) {
> + if (N2Undef)
> + MaskVec[i] = -1;
> + else
> + AllLHS = false;
> + } else if (MaskVec[i] >= 0) {
> + AllRHS = false;
> + }
Don't you also want to do something special if N1 is undef?
Also, you can do an early exit if both AllLHS and AllRHS are false.
> + // If Identity shuffle, or all shuffle in to undef, return that node.
> + bool AllUndef = true;
> + bool Identity = true;
> + for (int i = 0; i < NElts; ++i) {
> + if (MaskVec[i] >= 0 && MaskVec[i] != i) Identity = false;
> + if (MaskVec[i] >= 0) AllUndef = false;
> + }
You can do an early exit if both Identity and AllUndef are false.
> + // Blah?
> + Result->getTree(0)->setTransformFn(Pattern->getTree(0)->getTransformFn());
What's this bit about?
I didn't check the target specific code. Thanks again for doing this!
Ciao,
Duncan.
More information about the llvm-commits
mailing list