[llvm-commits] Patch for scalarized division.

[Mon Ping Wang]

Hi,

We use to have a design where LegalizeOp did both type legalization  
and operation legalization.  The problem with that approach was that  
after type legalization, there were opportunities for dag combiner to  
cleanup code and if it was one phase, we would miss these  
optimizations or have to teach legalization the optimizations that dag  
combiner has.

Here is a patch for an alternative approach based on using the custom  
lowering hook. The default widening algorithm doesn't know if an  
operation doesn't have a vector equivalent nor if the operation can  
trap since this is target dependent. For these cases, widen will want  
to scalarize the operation and rebuild the vector and add any undefs  
at the end for the widen vector.  This avoids generating the operation  
on undef. The patch adds the custom lowering node hook to Widening in  
Type Legalization.  In the target machine, for any vector divide/ 
remainder operation that we would need to widen (i.e., vectors of  
illegal types), we would call the custom widen operation which does  
the above.  I need to add a few test cases but it does fix the case  
you gave in your example.  Please let me know what you think.

-- Mon Ping





On Nov 7, 2009, at 7:55 AM, Sjodin, Jan wrote:

> I agree that this is not the ideal way to do things, and the list of  
> operations is indeed incomplete and may be different on different  
> targets.
> The problem is that the type legalization is done without taking the  
> operations into account. A more general solution would be to combine  
> the type and ops legalization into one pass. A somewhat simpler  
> solution could be to scalarize operations first, then do the type  
> legalization and finally apply the non-scalarizing legalize vector  
> ops. I do not know if there are any cases where legalization needs  
> more information about the surrounding operations to produce correct  
> code.
>
> - Jan
>
> From: Mon Ping Wang [mailto:wangmp at apple.com]
> Sent: Friday, November 06, 2009 11:59 PM
> To: Sjodin, Jan
> Cc: 'llvm-commits at cs.uiuc.edu'
> Subject: Re: [llvm-commits] Patch for scalarized division.
>
> Hi Jan,
>
> Some quick comments about the patch.
>
> @@ -282,6 +289,95 @@ SDValue VectorLegalizer::UnrollVSETCC(SD
>
> +bool VectorLegalizer::rangeIsDefined(SDValue Op,
> +                                   uint64_t LoBitIndex,
> +                                   uint64_t HiBitIndex)
> +{
> +[Deleted Code]
> +  case ISD::ADD:
> +  case ISD::SUB:
> +  case ISD::MUL:
> +  case ISD::SDIV:
> +  case ISD::UDIV:
> +  case ISD::SREM:
> +  case ISD::UREM: {
>
> We should expand the  list will have to expand to almost all  
> operators.  For example, an ISD::AND or ISD::XOR could cause the  
> same problem as well as VECTOR_SHUFFLE.
>
> @@ -326,6 +422,16 @@
> SDValue VectorLegalizer::UnrollVectorOp(
>        Scalars.push_back(DAG.getNode(Op.getOpcode(), dl, EltVT,  
> Operands[0],
>                                      DAG.getShiftAmountOperand 
> (Operands[1])));
>        break;
> +      // Ensure that both operands are defined, if not, the result  
> will
> +      // be undefined.
> +    case ISD::SDIV:
> +    case ISD::UDIV:
>
> For some machines, SREM and UREM could have the same issue.
>
> This is a generic issue that goes beyond just  SDIV/UDIV.  In the  
> majority of these cases, I think we should try to avoid generating  
> operations for UNDEF.  The only cases that give me pause of doing  
> this all the time are operations like multiplication by 0, and of 0,  
> etc.. where the UNDEF would be defined.  The only other thing is  
> that we recursively walk up an expression tree.  If the expression  
> tree is deep, this could cost some time.
>
> The main culprit that causes these UNDEF  is due to widening.  A  
> crazy idea to avoid doing this is if a hardware knows a particular  
> operation should not be widened, maybe it could use a target hook to  
> scalarize the operation early doing LegalizeTypes.  For X86, since  
> divides will not be vector operations, we would avoid widening them  
> only to lower them and work hard trying to avoid generating the  
> extra divides.
>
> -- Mon Ping
>
>
> On Nov 6, 2009, at 1:39 PM, Sjodin, Jan wrote:
>
>> In LegalizeVectorOps the resulting scalar divisions must not be
>> generated if they have undefined vector elements as operands. This  
>> may
>> be unsafe because the divides can cause division by zero exceptions,
>> and also slows down the code. This patch attempts to fix this issue  
>> by
>> checking if the operands are defined or not. I would be grateful if
>> someone could review this patch. Thanks!
>>
>> - Jan  
>> Sjodin 
>> < 
>> 0015_scalarized_div.diff 
>> >_______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20091112/84c9df7c/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: scalar_widen.patch
Type: application/octet-stream
Size: 5886 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20091112/84c9df7c/attachment.obj>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20091112/84c9df7c/attachment-0001.html>