[llvm-commits] Patch for scalarized division.

[Sjodin, Jan]

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<mailto: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/20091107/05655274/attachment.html>