LGTM.

LGTM

This change LGTM. Lets wait for a few days to give others time to take a look as well.

Commit 6db314e86b26741c2e29ce51d88a6a5dca35336c causes following failure:

LGTM

This refactoring looks reasonable to me. I've left some comments inline.

Since the InjectTLIMappings pass impacts mappings of scalar math functions to their vector counterparts in all three of the currently supported vector libraries, I would like to suggest adding a test case for each one of them. In other words, testing in the presence of -fveclib=Accelerate, -fveclib=SVML, and -fveclib=MASSV libraries.

@jsji @nemanjai I dont have commit access, could you please commit this patch for me? Thanks.

@jsji Using setLibcallName to override the generic fun*_massv names with target specific names is feasible. However, it entails extension of ISDNodes to support vector types as well. Furthermore, with the current infrastructure, distinguishing MASSV calls from a list of scalar math, and other runtime library calls will incur the overheads of look up and comparison.

In D59883#1649656, @jsji wrote:

1. *_massv functions serve as place holder for math functions during loop vectorization. They are not actual entries in the MASS library. I think lowering them to actual entries in the MASS library deserves a separate pass rather than polluting the TargetLowering.cpp, which is already big as it is.

Why this has to be in TargetLowering.cpp, they can be in PPCISelLowering.cpp.

2. If we were to use setLibcallName, we would need to repeat the list of all supported MASS entries in TargetLowering as well. With the current structure, we are reusing the list in VecFuncs.def. Also, this structure sits well with vectorization of math functions with other libraries too, namely: SVML and Accelerate.

Why we can't reusing the list in VecFuncs.def with setLibcallName?

In D59883#1649479, @jsji wrote:

A separate pass for MASSV lowering is worthwhile because it allows better separation of concerns.
In the future, if need be, other PPC sub-target specific lowering decisions for MASSV may be encapsulated in this pass too.

What is the benefits other than allows better separation of concerns?
I don't see this compelling reason to be a pass, especially considering the unnecessary compile time it will cost.

Addressed review comments in this diff:

1. use -mattr instead of relying attributes on the caller; separate the tests to a separate .ll file
2. add -verify-machineinstrs to the run line
3. combine multiple check-prefix into one check-prefixes
4. remove datalayout and triple from the .ll file

In D59883#1605289, @jsji wrote:

Why we need a new pass to do this simple lowering?
Why can't we do it similar to InitLibcalls and change the suffixes for subtarget using setLibcallName?

Re-basing this patch off of the patch in https://reviews.llvm.org/D59881

ping @nemanjai @jsji

In D59881#1530258, @nemanjai wrote:

The LLVM portion of this patch committed in https://reviews.llvm.org/rL362568.

This patch includes following updates:

1. sqrt (f32 and f64) intrinsics are lowered to their vector intrinsic counterparts; lowering them to massv entries is beyond the scope of this patch. Relevant tests for the sqrt intrinsics are added to massv-unsupported.ll
2. The test files include comments to portray their intent.

Thank you for your comments @jsji
This patch includes following updates:

• negative tests for math functions lacking vector counterparts, for altivec targets, and for nobuiltin attributes.

Updated the patch assuming a base that has D60211 incorporated.

ping

This updated patch uses a single macro to define the mapping from scalar maths functions to their vector counterparts ( as suggested in the earlier review of the original patch).