<html><head></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space; "><div>Hi Jan,</div><div><br></div><div>Some quick comments about the patch.</div><div><br></div><div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">@@ -282,6 +289,95 @@ SDValue VectorLegalizer::UnrollVSETCC(SD</span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; "><br></span></font></div></div><div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">+bool VectorLegalizer::rangeIsDefined(SDValue Op, </span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">+                                   uint64_t LoBitIndex,  </span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">+                                   uint64_t HiBitIndex)</span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">+{</span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">+[Deleted Code]</span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">+  case ISD::ADD: </span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">+  case ISD::SUB:</span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">+  case ISD::MUL:</span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">+  case ISD::SDIV:</span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">+  case ISD::UDIV:</span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">+  case ISD::SREM:</span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">+  case ISD::UREM: {</span></font></div></div><div><br></div><div>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. </div><div><br></div><div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">@@ -326,6 +422,16 @@</span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">SDValue VectorLegalizer::UnrollVectorOp(</span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">       Scalars.push_back(DAG.getNode(Op.getOpcode(), dl, EltVT, Operands[0],</span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">                                     DAG.getShiftAmountOperand(Operands[1])));</span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">       break;</span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">+      // Ensure that both operands are defined, if not, the result will</span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">+      // be undefined. </span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">+    case ISD::SDIV:</span></font></div><div><font class="Apple-style-span" face="Courier" size="3"><span class="Apple-style-span" style="font-size: 12px; ">+    case ISD::UDIV:</span></font></div></div><div><br></div><div>For some machines, SREM and UREM could have the same issue.</div><div><br></div><div>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.</div><div><br></div><div>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.</div><div><br></div><div>-- Mon Ping</div><div><br></div><br><div><div>On Nov 6, 2009, at 1:39 PM, Sjodin, Jan wrote:</div><br class="Apple-interchange-newline"><blockquote type="cite"><div>In LegalizeVectorOps the resulting scalar divisions must not be<br>generated if they have undefined vector elements as operands. This may<br>be unsafe because the divides can cause division by zero exceptions,<br>and also slows down the code. This patch attempts to fix this issue by<br>checking if the operands are defined or not. I would be grateful if<br>someone could review this patch. Thanks!<br><br>- Jan Sjodin<span><0015_scalarized_div.diff></span>_______________________________________________<br>llvm-commits mailing list<br><a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br><a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br></div></blockquote><div><br></div></div><div><div><br></div></div></body></html>