<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi Michael,<div class=""><br class=""></div><div class="">thanks for following up on this. I think it is important to move forward with this.</div><div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Feb 26, 2015, at 12:01 PM, Michael Zolotukhin <<a href="mailto:mzolotukhin@apple.com" class="">mzolotukhin@apple.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><meta http-equiv="Content-Type" content="text/html charset=utf-8" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Hi,<div class=""><br class=""></div><div class="">Some time ago James posted a patch for vectorization of calls [1]. Though it seems to be a missing feature in the vectorizer, it wasn’t committed, so I decided to revive the discussion.</div><div class=""><br class=""></div><div class="">To begin with, I rebased the patch and split it to several semantically separated parts. The approach used here is pretty natural: TargetLibraryInfo has a mapping for a set of scalar functions to their vector counterparts, TargetTransformInfo consults with it when cost of call instruction is calculated, if the vector-version is cheaper, vectorizer emits vector-function.</div><div class=""><br class=""></div><div class="">There are several question on this patch:</div><div class="">1. Is this approach still viable in general and worth pursuing?</div></div></div></blockquote><div><br class=""></div>I believe this is still important. We want to be able to vectorize function calls if vectorized versions of math library functions are available.</div><div><br class=""></div><div><br class=""><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">2. The cost model is changed. In theory it should become more accurate, but I haven’t measured performance impact of this change yet. Does these changes look good in general? If yes, do we need to tune some target-cost-model parameters, or those are the values that no one has been actually using yet?</div></div></div></blockquote><div><br class=""></div><div>I think we will have to test performance impact of this change since we are most likely the first clients. We should test with/without the target tuple that enables the vectorized function calls. I think if we don’t see egregious regressions in the run that uses the modified cost model but is a run without vectorized versions of the math library being available we can commit this patch set such that it gets testing by the community.</div><div><br class=""></div><div><br class=""></div><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class="">3. What is the best way to populate the scalar to vector functions map? For now, I populate it when the target-triple contains ‘Accelerate’. If we have a lot of other cases, we might think of something different here.</div></div></div></blockquote><div><br class=""></div>We use the environment substring of the target tuple (in this case quadruple). This is in spirit to what we are already to conditionally turn on/off features in TargetLibraryInfo. I think we should address the issue of scalability when we have a client where this gets to be an issue.</div><div><br class=""></div><div><br class=""></div><div><blockquote type="cite" class=""><div class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""><br class=""></div><div class="">Any feedback or ideas are appreciated!</div><div class=""><br class=""></div><div class="">Thanks,</div><div class="">Michael</div><div class=""><br class=""></div><div class="">[1]: <a href="http://thread.gmane.org/gmane.comp.compilers.llvm.cvs/169736" class="">http://thread.gmane.org/gmane.comp.compilers.llvm.cvs/169736</a></div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""></div></div><span id="cid:72C9864E-B093-4908-B077-E26C83CD717A@apple.com"><0001-Make-ToVectorTy-static.patch></span><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><meta http-equiv="Content-Type" content="text/html charset=us-ascii" class=""><div class=""></div></div><span id="cid:F3E4FF9B-FA63-49D4-99B4-D4C4C2A9011D@apple.com"><0002-Fix-a-copy-paste-bug.patch></span><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><meta http-equiv="Content-Type" content="text/html charset=us-ascii" class=""><div class=""></div></div><span id="cid:8C2AE955-87AD-4C6D-8FFC-E33C0CDE2025@apple.com"><0003-TLI-Use-lambda.-NFC.patch></span><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><meta http-equiv="Content-Type" content="text/html charset=us-ascii" class=""><div class=""></div></div><span id="cid:1271C1E2-A213-46C6-833D-E7F4F95D921A@apple.com"><0004-LoopVectorizer-Add-TargetTransformInfo.patch></span><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><meta http-equiv="Content-Type" content="text/html charset=us-ascii" class=""><div class=""></div></div><span id="cid:10AF974B-E24C-47DD-8B86-AEBCD1C454C3@apple.com"><0005-TLI-Factor-out-sanitizeFunctionName.-NFC.patch></span><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><meta http-equiv="Content-Type" content="text/html charset=us-ascii" class=""><div class=""></div></div><span id="cid:15AF3830-1C84-49D2-BB51-2DF8A6F87ED6@apple.com"><0006-TLI-Add-interface-for-querying-whether-a-function-is.patch></span><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><meta http-equiv="Content-Type" content="text/html charset=us-ascii" class=""><div class=""></div></div><span id="cid:A97C03FA-C950-4BA5-8016-C7FF76DD6A58@apple.com"><0007-TTI-Add-getCallInstrCost.patch></span><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><meta http-equiv="Content-Type" content="text/html charset=us-ascii" class=""><div class=""></div></div><span id="cid:EECD50FA-D49F-4131-BD52-D52D1CCBCEC0@apple.com"><0008-LoopVectorize-teach-loop-vectorizer-to-vectorize-cal.patch></span><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><meta http-equiv="Content-Type" content="text/html charset=us-ascii" class=""><div class=""></div></div><span id="cid:E4B0D28F-4DC2-4981-9A80-663D4CBCA5E9@apple.com"><0009-TTI-Honour-cost-model-for-estimating-cost-of-vector-.patch></span><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><meta http-equiv="Content-Type" content="text/html charset=us-ascii" class=""><div class=""></div></div><span id="cid:AA4F87BF-D1C9-4C6E-B9DD-5FA84F41C58B@apple.com"><0010-TLI-Add-several-entries-to-VectorizableFunctions-tab.patch></span><meta http-equiv="Content-Type" content="text/html charset=us-ascii" class=""><div style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class=""><div class=""></div></div></div></blockquote><br class=""></div><div>Some comments on the code:</div><div><br class=""></div><div><br class=""></div><div><div>This function could use some documentation. What are Tys? (Argument types).</div><div><br class=""></div><div><div>+ unsigned getCallInstrCost(Function *F, Type *RetTy, ArrayRef<Type *> Tys,</div><div>+ const TargetLibraryInfo *TLI) {</div><div>+</div><div>+ // Scalar function calls are always expensive.</div><div>+ if (!RetTy->isVectorTy())</div><div>+ return 10;</div><div>+</div><div>+ // Functions with a vector form are no more expensive than a scalar call.</div><div>+ if (TLI &&</div><div>+ TLI->isFunctionVectorizable(F->getName(),</div><div>+ RetTy->getVectorNumElements()))</div><div>+ return 10;</div><div>+</div><div>+ // We have to scalarize this function call. Estimate the cost.</div><div>+ unsigned ScalarizationCost = getScalarizationOverhead(RetTy, true, false);</div><div>+ unsigned ScalarCalls = RetTy->getVectorNumElements();</div><div>+ for (unsigned i = 0, ie = Tys.size(); i != ie; ++i) {</div><div>+ if (Tys[i]->isVectorTy()) {</div><div>+ ScalarizationCost += getScalarizationOverhead(Tys[i], false, true);</div><div><br class=""></div><div>This is not obvious. Why could we have more elements in the vector type of a function arguments than the vector return type when vectorizing a function such that we need more function calls when scalarizing?</div><div><br class=""></div><div>+ ScalarCalls = std::max(ScalarCalls, Tys[i]->getVectorNumElements());</div><div>+ }</div><div>+ }</div><div>+</div><div>+ return ScalarCalls * 10 + ScalarizationCost;</div><div>+ }</div></div><div><br class=""></div><div><br class=""></div><div>There seems to be some repetition in the code below. Can we refactor that?</div><div><br class=""></div><div><div>+ default:</div><div>+ bool HasScalarOpd = hasVectorInstrinsicScalarOpd(ID, 1);</div><div>+ for (unsigned Part = 0; Part < UF; ++Part) {</div><div>+ SmallVector<Value *, 4> Args;</div><div>+ for (unsigned i = 0, ie = CI->getNumArgOperands(); i != ie; ++i) {</div><div>+ if (HasScalarOpd && i == 1) {</div><div>+ Args.push_back(CI->getArgOperand(i));</div><div>+ continue;</div><div>+ }</div><div>+ VectorParts &Arg = getVectorValue(CI->getArgOperand(i));</div><div>+ Args.push_back(Arg[Part]);</div><div>+ }</div><div>+ Type *Tys[] = {CI->getType()};</div><div>+ if (VF > 1)</div><div>+ Tys[0] = VectorType::get(CI->getType()->getScalarType(), VF);</div><div>+</div><div>+ Function *F = Intrinsic::getDeclaration(M, ID, Tys);</div><div>+ Entry[Part] = Builder.CreateCall(F, Args);</div><div>+ }</div><div>+</div><div>+ propagateMetadata(Entry, it);</div><div>+ break;</div><div>+ }</div><div>+ // Otherwise, perform the lib call.</div><div>+ } else if (TLI && TLI->isFunctionVectorizable(FnName, VF)) {</div><div>+ // This is a function with a vector form.</div><div>+ StringRef VFnName = TLI->getVectorizedFunction(FnName, VF);</div><div>+ assert(!VFnName.empty());</div><div>+</div><div>+ Function *VectorF = M->getFunction(VFnName);</div><div>+ if (!VectorF) {</div><div>+ // Generate a declaration</div><div>+ FunctionType *FTy = FunctionType::get(RetTy, Tys, false);</div><div>+ VectorF =</div><div>+ Function::Create(FTy, Function::ExternalLinkage, VFnName, M);</div><div>+ assert(VectorF);</div><div>+ }</div><div>+</div><div> for (unsigned Part = 0; Part < UF; ++Part) {</div><div> SmallVector<Value *, 4> Args;</div><div> for (unsigned i = 0, ie = CI->getNumArgOperands(); i != ie; ++i) {</div><div>- if (HasScalarOpd && i == 1) {</div><div>- Args.push_back(CI->getArgOperand(i));</div><div>- continue;</div><div>- }</div><div> VectorParts &Arg = getVectorValue(CI->getArgOperand(i));</div><div> Args.push_back(Arg[Part]);</div><div> }</div><div>- Type *Tys[] = {CI->getType()};</div><div>- if (VF > 1)</div><div>- Tys[0] = VectorType::get(CI->getType()->getScalarType(), VF);</div><div>-</div><div>- Function *F = Intrinsic::getDeclaration(M, ID, Tys);</div><div>- Entry[Part] = Builder.CreateCall(F, Args);</div><div>+ Entry[Part] = Builder.CreateCall(VectorF, Args);</div><div> }</div><div class=""><br class=""></div></div><div><br class=""></div></div><br class=""></div></body></html>