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

Renato Golin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 27 10:37:44 PST 2019


rengolin added a comment.

In D53927#1412003 <https://reviews.llvm.org/D53927#1412003>, @joelkevinjones wrote:

> I'm not sure what the proposed split would consist of.


I can see a few different concepts introduced here:

- Lowering sin/cos/tan, etc and all related mechanical changes (large)
- Adding the STRICT variants, with aliases on the lowering switches (medium)
- Changing the log10 behaviour and code movement (small)
- Adding SLEEF at the end, making use of all three patches above (tiny)

The lines between those concepts may be blurred, so it's never a clear cut, and trying to split the patch in that sequence may show problems I haven't consider.

But Git is quite helpful in splitting patches into a branch, and then testing the branch, and then one patch at a time.

>From a high point of view, I can't see why any intermediate state would be harmful to builds or tests, given they only add unused code, which will be enabled by the last one.

I also imagine the tests can only work after the last patch is applied, and that's the main reason why I was ok with the patch being a single bulk.

The other reason is because there's a Clang patch, which needs to be applied after all these, and it may cause bisect issues to have too many depending patches across repos.

Hopefully the monorepo will help solve that problem, but we're not there yet.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53927





More information about the llvm-commits mailing list