[PATCH] D53927: [AArch64] Enable libm vectorized functions via SLEEF

[Stefan Teleman via Phabricator via llvm-commits]

steleman added a comment.

In D53927#1400072 <https://reviews.llvm.org/D53927#1400072>, @bryanpkc wrote:

> In D53927#1399283 <https://reviews.llvm.org/D53927#1399283>, @steleman wrote:
>
> > In D53927#1399263 <https://reviews.llvm.org/D53927#1399263>, @rengolin wrote:
> >
> > > Thanks Stephan, now looks good.
> > >
> > > Did you see the comment on lgamma coming after isoc99? I wonder if that's reproducible in all systems...
> >
> >
> > Yup I saw it. But I don't get any errors here (Ubuntu 16.04 / Ubuntu 18.something) AArch64.
> >
> > And it's not clear to me how that sorted alphabetical order is supposed to work. There's a bunch of other functions that come after lgamma/tgamma that aren't sorted.
> >
> > For example:
> >
> >   /// char * __strtok_r(char *s, const char *delim, char **save_ptr);
> >   TLI_DEFINE_ENUM_INTERNAL(dunder_strtok_r)
> >   TLI_DEFINE_STRING_INTERNAL("__strtok_r")
> >   /// int abs(int j);
> >   TLI_DEFINE_ENUM_INTERNAL(abs)
> >   TLI_DEFINE_STRING_INTERNAL("abs")
> >   /// int access(const char *path, int amode);
> >   TLI_DEFINE_ENUM_INTERNAL(access)
> >   TLI_DEFINE_STRING_INTERNAL("access")
> >
>
>
> The build was fine, but when running the compiler, this assertion in `lib/Analysis/TargetLibraryInfo.cpp` will fail:
>
>   static void initialize(TargetLibraryInfoImpl &TLI, const Triple &T,
>                          ArrayRef<StringRef> StandardNames) {
>     // Verify that the StandardNames array is in alphabetical order.
>     assert(std::is_sorted(StandardNames.begin(), StandardNames.end(),
>                           [](StringRef LHS, StringRef RHS) {
>                             return LHS < RHS;
>                           }) &&
>            "TargetLibraryInfoImpl function names must be sorted");
>
>
> You probably don't see it because you are only building in Release mode.
>
> P.S. The examples you listed are sorted (`_` is less than `a`).


I think the correct fix here is to use an associative container, not an array, and not rely on writing a text file containing sorted character strings.

std::set comes to mind.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D53927/new/

https://reviews.llvm.org/D53927