search for: isoperationlegalorcustom

I found a bug in the expansion code for SUB going back to at least LLVM 2.6 and still shows up in trunk. case ISD::SUB: { EVT VT = Node->getValueType(0); assert(TLI.isOperationLegalOrCustom(ISD::ADD, VT) && TLI.isOperationLegalOrCustom(ISD::XOR, VT) && "Don't know how to expand this subtraction!"); Tmp1 = DAG.getNode(ISD::XOR, dl, VT, Node->getOperand(1), DAG.getConstant(APInt::getAllOnesValue(VT.getSizeInBits()...

...(*ext y))) -> // (*ext (rotl x, y)) // fold (or (shl (*ext x), (*ext (sub 32, y))), // (srl (*ext x), (*ext y))) -> // (*ext (rotr x, (sub 32, y))) In order to do this, code checks whether target supports this dag operation with specific type(LHS's type) through "TLI.isOperationLegalOrCustom" function. I guess the reason is following "LegalizeType" can not promote "ROTL" and "ROTR". The problem is that code does not check this with new specific type "LArgVT" and "RArgVT" while processing above patterns. How do you think about t...

> Historically nodes marked "custom" were considered legal, so the > DAGCombiner would have been correct to generate it. Not sure how > that ever worked though. I think Dan split the isOperationLegal > method into isOperationLegal and isOperationLegalOrCustom for reasons > related to this kind of thing. I don't know whether the DAGCombiner > is now only supposed to produce legal non-custom nodes. In my understanding, "Custom" means that I'm in charge to build the node. So, "Custom" means "Legal" once the node...

...e lowered using two-result operations. // We just need the low half of the multiply; try both the signed // and unsigned forms. If the target supports both SMUL_LOHI and // UMUL_LOHI, form a preference by checking which forms of plain // MULH it supports. bool HasSMUL_LOHI = TLI.isOperationLegalOrCustom(ISD::SMUL_LOHI, VT); bool HasUMUL_LOHI = TLI.isOperationLegalOrCustom(ISD::UMUL_LOHI, VT); bool HasMULHS = TLI.isOperationLegalOrCustom(ISD::MULHS, VT); bool HasMULHU = TLI.isOperationLegalOrCustom(ISD::MULHU, VT); unsigned OpToUse = 0; if (HasSMUL_LOHI && !HasMULHS) {...

Hi Gabrielle, > > Historically nodes marked "custom" were considered legal, so the > > DAGCombiner would have been correct to generate it. Not sure how > > that ever worked though. I think Dan split the isOperationLegal > > method into isOperationLegal and isOperationLegalOrCustom for reasons > > related to this kind of thing. I don't know whether the DAGCombiner > > is now only supposed to produce legal non-custom nodes. > In my understanding, "Custom" means that I'm in charge to build the node. So, "Custom" means "Legal&quot...

On May 21, 2012, at 11:21 AM, Villmow, Micah wrote: > I found a bug in the expansion code for SUB going back to at least LLVM 2.6 and still shows up in trunk. > case ISD::SUB: { > EVT VT = Node->getValueType(0); > assert(TLI.isOperationLegalOrCustom(ISD::ADD, VT) && > TLI.isOperationLegalOrCustom(ISD::XOR, VT) && > "Don't know how to expand this subtraction!"); > Tmp1 = DAG.getNode(ISD::XOR, dl, VT, Node->getOperand(1), > DAG.getConstant(APInt::getAllOnesValu...

...4-bit operation into 32-bit one. > > I am suspecting the code in SelectionDAGLegalize won't expand 64-bit operations to 32-bit ones. For example, I see this code in SelectionDAGLegalize::ExpandNode (near line 3090): case ISD::SUB: { EVT VT = Node->getValueType(0); assert(TLI.isOperationLegalOrCustom(ISD::ADD, VT) && TLI.isOperationLegalOrCustom(ISD::XOR, VT) && "Don't know how to expand this subtraction!"); If we mark the action of SUB, ADD and XOR as 'Expand', the code will assert. So you have to either make i64 illegal or mark th...

...ons. > My questions: > 1) Is my understanding right? Historically nodes marked "custom" were considered legal, so the DAGCombiner would have been correct to generate it. Not sure how that ever worked though. I think Dan split the isOperationLegal method into isOperationLegal and isOperationLegalOrCustom for reasons related to this kind of thing. I don't know whether the DAGCombiner is now only supposed to produce legal non-custom nodes. > 2) Is there any bug report generated after the discussion below? I don't know, sorry. Ciao, Duncan.

...== ISD::SHL) { <== This branch is taken if (Amt > VTBits) { <== False ... not executed ... } else if (Amt > NVTBits) { <== False ... not executed ... } else if (Amt == NVTBits) { <== False ... not executed ... } else if (Amt == 1 && TLI.isOperationLegalOrCustom(ISD::ADDC, TLI.getTypeToExpandTo(*DAG.getContext(), NVT))) { <== False ... not executed ... } else { <== This branch is taken Lo = DAG.getNode(ISD::SHL, dl, NVT, InL, DAG.getConstant(Amt, ShTy)); <== Source low part is shifted left by 6 bits Hi = DAG.getNode(ISD::OR, d...

...ned Align = ST->getAlignment(); MVT SVT = Value.getOperand(0).getValueType(); unsigned OrigAlign = TLI.getTargetData()-> getABITypeAlignment(SVT.getTypeForMVT()); if (Align <= OrigAlign && ((!LegalOperations && !ST->isVolatile()) || TLI.isOperationLegalOrCustom(ISD::STORE, SVT))) return DAG.getStore(Chain, N->getDebugLoc(), Value.getOperand(0), Ptr, ST->getSrcValue(), ST->getSrcValueOffset(), ST->isVolatile(), OrigAlign); } Uhh...this doesn't seem legal to me. How can we just w...

...eachable("Unknown FP type"); case MVT::f80: // We don't do this for these yet. case MVT::f128: case MVT::ppcf128: break; case MVT::f32: if ((isTypeLegal(MVT::i32) && !LegalOperations && !ST->isVolatile()) || TLI.isOperationLegalOrCustom(ISD::STORE, MVT::i32)) { Tmp = DAG.getConstant((uint32_t)CFP->getValueAPF(). bitcastToAPInt().getZExtValue(), MVT::i32); return DAG.getStore(Chain, N->getDebugLoc(), Tmp, Ptr, ST->getPointerInfo(), ST->isVol...

Hi Duncan, The problem is the implementation of the expansion. Perhaps an example can help illustrate better. Take the case of a 64-bit integer shifted left by say 6 bits and is decomposed using 32-bit registers. Because 6 is less than the 32 (the register size) the resulting low part should be equal to the source low part shifted left by 6 bits. The current implementation places a zero

Hello, In the combine 2 step (after legalization), in the DAGCombiner::visitBIT_CONVERT() method, the DAG combiner is replacing an FABS followed by a BIT_CONVERT, to a BIT_CONVERT followed by an AND 0x7FFFFFFFFFFFFFFF. Everything is 64 bit. On my target, FABS and BIT_CONVERT are legal in 64 bit, but AND in not legal in 64 bit (is declared custom). So the dag combiner is introducing illegal (not

On Thu, Sep 6, 2012 at 10:02 AM, Reed Kotler <rkotler at mips.com> wrote: > Here is the problem explained more. > > Normally there is a 64 bit register that is the result of certain multiply > and divide instructions. > It's really 2 32 bit registers. > > This is like HI[0]/Lo[0] > > In fact there are four such pairs, only the 0th pair available to basic >

...t(); > MVT SVT = Value.getOperand(0).getValueType(); > unsigned OrigAlign = TLI.getTargetData()-> > getABITypeAlignment(SVT.getTypeForMVT()); > if (Align <= OrigAlign && > ((!LegalOperations && !ST->isVolatile()) || > TLI.isOperationLegalOrCustom(ISD::STORE, SVT))) > return DAG.getStore(Chain, N->getDebugLoc(), Value.getOperand(0), > Ptr, ST->getSrcValue(), > ST->getSrcValueOffset(), ST->isVolatile(), > OrigAlign); > } > > Uhh...this doesn't see...

I have added a few PPC-specific DAG combines in the past that follow this pattern on specific operations. Now that it appears that this would be useful to do on yet another operation, I'm wondering what people think about doing this in the target-independent DAG Combiner for any legal/custom operation on the target. TL; DR; The generic pattern would look like this: (build_vector (op

...ken > if (Amt > VTBits) { <== False > ... not executed ... > } else if (Amt > NVTBits) { <== False > ... not executed ... > } else if (Amt == NVTBits) { <== False > ... not executed ... > } else if (Amt == 1 && > TLI.isOperationLegalOrCustom(ISD::ADDC, > TLI.getTypeToExpandTo(*DAG.getContext(), NVT))) { <== False > ... not executed ... > } else { <== This branch is taken > Lo = DAG.getNode(ISD::SHL, dl, NVT, InL, DAG.getConstant(Amt, > ShTy)); <== > Source low part is shifted left by 6 bit...

...torization in the SelectionDAG tends to be fraught with > problems as we don't have a cost model mechanism to control it; we hit this > in https://bugs.llvm.org/show_bug.cgi?id=35732 where conversion ops were > being vectorized in DAG whether we wanted to or not, purely based on a > isOperationLegalOrCustom call (just because an op is possible doesn't mean > its efficient to do so under all circumstances). > > In general I think we should be relying on the SLPVectorizer to handle > this instead. If SLP is failing, we're probably better off spending dev > time fixing it there ins...

...ends to be fraught with >>> problems as we don't have a cost model mechanism to control it; we hit this >>> in https://bugs.llvm.org/show_bug.cgi?id=35732 where conversion ops >>> were being vectorized in DAG whether we wanted to or not, purely based on a >>> isOperationLegalOrCustom call (just because an op is possible doesn't mean >>> its efficient to do so under all circumstances). >>> >>> In general I think we should be relying on the SLPVectorizer to handle >>> this instead. If SLP is failing, we're probably better off spending d...

Here is the problem explained more. Normally there is a 64 bit register that is the result of certain multiply and divide instructions. It's really 2 32 bit registers. This is like HI[0]/Lo[0] In fact there are four such pairs, only the 0th pair available to basic multiply and divide. But DSP instructions have access to 4 , Hi[i],Lo[i], i=0..3 We want the register allocator to allocate