[PATCH] TLI: Add interface for querying whether a function is vectorizable.

[Hal Finkel]

----- Original Message -----
> From: "Arnold" <aschwaighofer at apple.com>
> To: reviews+D8093+public+393a2968a4d99986 at reviews.llvm.org
> Cc: mzolotukhin at apple.com, hfinkel at anl.gov, nrotem at apple.com, "james molloy" <james.molloy at arm.com>,
> llvm-commits at cs.uiuc.edu
> Sent: Friday, March 6, 2015 3:59:47 AM
> Subject: Re: [PATCH] TLI: Add interface for querying whether a function is vectorizable.
> 
> Sent from my iPhone
> 
> > On Mar 5, 2015, at 8:36 PM, "hfinkel at anl.gov" <hfinkel at anl.gov>
> > wrote:
> > 
> > ================
> > Comment at: lib/Analysis/TargetLibraryInfo.cpp:758
> > @@ -753,1 +757,3 @@
> > 
> > +static bool compareByScalarFnName(const VecDesc &LHS, const
> > VecDesc &RHS) {
> > +  return std::strncmp(LHS.ScalarFnName, RHS.ScalarFnName,
> > ----------------
> > It looks like all of these comparison functions are only used once.
> > Please make them lambdas at their current call sites.
> 
> 
> Why? He had this in his original patch. In my review I argued that
> using static function makes it obvious what is going on. Especially,
> since the body of the lambdas look very similar.
> 
> Compare:
> 
> std::sort(VectorDescs.begin(), VectorDescs.end(),
> compareByScalarName)
> 
> std::sort(ScalarDescs.begin(), ScalarDescs.end(),
> compareByVectorName)
> 
> std::lower_bound(VectorDescs.begin(), VectorDescs.end(),
> FunctionName, compareScalarNameWith)
> 
> etc.
> 
> +void
> TargetLibraryInfoImpl::addVectorizableFunctions(ArrayRef<VecDesc>
> Fns) {
> +  VectorDescs.insert(VectorDescs.end(), Fns.begin(), Fns.end());
> +  std::sort(VectorDescs.begin(), VectorDescs.end(),
> +            [](const VecDesc &LHS, const VecDesc &RHS) {
> +              return std::strncmp(LHS.ScalarFnName,
> RHS.ScalarFnName,
> +                                  std::strlen(RHS.ScalarFnName)) <
> 0;
> +            });
> +
> +  ScalarDescs.insert(ScalarDescs.end(), Fns.begin(), Fns.end());
> +  std::sort(ScalarDescs.begin(), ScalarDescs.end(),
> +            [](const VecDesc &LHS, const VecDesc &RHS) {
> +              return std::strncmp(LHS.VectorFnName,
> RHS.VectorFnName,
> +                                  std::strlen(RHS.VectorFnName)) <
> 0;
> +            });
> +}
> +
> +bool TargetLibraryInfoImpl::isFunctionVectorizable(StringRef
> funcName) const {
> +  funcName = sanitizeFunctionName(funcName);
> +  if (funcName.empty())
> +    return false;
> +
> +  std::vector<VecDesc>::const_iterator I = std::lower_bound(
> +      VectorDescs.begin(), VectorDescs.end(), funcName,
> +      [](const VecDesc &LHS, StringRef S) {
> +        return std::strncmp(LHS.ScalarFnName, S.data(), S.size()) <
> 0;
> +      });
> +  return I != VectorDescs.end();
> +}
> +
> +StringRef TargetLibraryInfoImpl::getVectorizedFunction(StringRef F,
> +                                                       unsigned VF)
> const {
> +  F = sanitizeFunctionName(F);
> +  if (F.empty())
> +    return F;
> +  std::vector<VecDesc>::const_iterator I = std::lower_bound(
> +      VectorDescs.begin(), VectorDescs.end(), F,
> +      [](const VecDesc &LHS, StringRef S) {
> +        return std::strncmp(LHS.ScalarFnName, S.data(), S.size()) <
> 0;
> +      });
> +  while (I != VectorDescs.end() && StringRef(I->ScalarFnName) == F)
> {
> +    if (I->VectorizationFactor == VF)
> +      return I->VectorFnName;
> +    ++I;
> +  }
> +  return StringRef();
> +}
> 
> 
> I don't think we should/ must use lambdas   (If there is just a
> single use) at the cost of readability.
> 
> compareWithScalarFnName is used in two places.

Okay, fair enough. Let's leave it as-is.

 -Hal

> 
> > http://reviews.llvm.org/D8093
> > 
> > EMAIL PREFERENCES
> > http://reviews.llvm.org/settings/panel/emailpreferences/
> > 
> > 
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory