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

[Francesco Petrogalli via Phabricator via llvm-commits]

fpetrogalli added a comment.

Hi @steleman , thank you for your patience.

> I don't understand this. Are you suggesting that decreasing the size of the statistical sample increases the accuracy?

yes, reducing the size of the array increases the accuracy of the measurement, if what you are measuring is latency of the function. You should also run each measurement multiple times and get the best.

> At any rate, the test benchmark and the results spreadsheet aren't part of this changeset.
>  So I'm confused as to what's being asked here.

Yes, the benchmarks and the results are not part of the submission. I think your results are quite unstable, I was suggesting changes to get them more stable (`sin` shouldn't differ so much from `cos`, because their algorithm is very similar). If I may, I would like to point you at the open source benchmarks that our math library (libm) team uses: https://github.com/ARM-software/optimized-routines/blob/master/test/mathbench.c#L253-L274

Having said that, the implementation of the functionality looks good to me. My only concerns are about the actual performance, that is why I was suggesting to use the 3.5 ULP version of the functions. But I'll let you sort out which version is best for you.

As per submitting this code, I have to redirect you to the maintainer of this part of the back-end for submitting your changes.

Kind regards,

Francesco


https://reviews.llvm.org/D53927