Initial support for vectorization using MASSV (IBM MASS vector library)

[pjeeva01]

Summary

This patch (first of two patches) enables the vectorizer to recognize the IBM MASS vector library routines. This patch specifically adds support for recognizing the “-vector-library=MASSV” option, and defines mappings from IEEE standard scalar math functions to generic PowerPC MASS vector counterparts.
For instance, the generic PowerPC MASS vector entry for double-precision “cbrt” function is “__cbrtd2_massv”

The second patch will further lower the generic PowerPC vector entries to PowerPC subtarget-specific entries.
For instance, the PowerPC generic entry “cbrtd2_massv” is lowered to “cbrtd2_P9” or Power9 subtarget.

The overall support for MASS vector library is presented as such in two patches for ease of review.

[jsji]

llvm/lib/Analysis/TargetLibraryInfo.cpp

1467 ↗

(On Diff #192443)

Can we split the refactoring of these definitions into llvm/Analysis/VecFuncs.def into a NFC patch first?
And leave only the MASSV related change in this patch ?

[pjeeva01]

llvm/lib/Analysis/TargetLibraryInfo.cpp

1467 ↗

(On Diff #192443)

@jsji Thank you for looking into this.

@mmasten @mzolotukhin If you agree with this refactoring suggestion, I will go ahead and create an NFC patch for the same.

[pjeeva01]

llvm/lib/Analysis/TargetLibraryInfo.cpp

1467 ↗

(On Diff #192443)

NFC patch that simply factors out these definitions into llvm/Analysis/VecFuncs.def is posted [[ llvm/Analysis/VecFuncs.def | here ]]

[pjeeva01]

ping

[hubert.reinterpretcast]

@pjeeva01, please post the delta assuming a base that has D60211 incorporated.

llvm/lib/Analysis/TargetLibraryInfo.cpp

1467 ↗

(On Diff #192443)

You mean, posted as D60211?

[pjeeva01]

Updated the patch assuming a base that has D60211 incorporated.

[jsji]

llvm/include/llvm/Analysis/VecFuncs.def

76 ↗

(On Diff #194193)

How about the finite versions? eg: __expf_finite

llvm/test/Transforms/LoopVectorize/PowerPC/massv-calls.ll

1 ↗

(On Diff #194193)

do we require any attr? eg: vsx? what happen to ppc without altivec or vsx?

4 ↗

(On Diff #194193)

How about powerpc (32 bit) or powerpc64?

93 ↗

(On Diff #194193)

It would be better if we check all the key elements of call , not only the name .
eg: call <2 x double> @__cbrtd2_massv(<2 x double> %2)

282 ↗

(On Diff #194193)

Missing some intrinsics test? @sqrt_f64_intrinsic?@sqrt_f32_intrinsic?

1524 ↗

(On Diff #194193)

Shall we add some negative tests to make sure that we don't vectorize functions not in the list, eg: arbitrary functions, function in ACCELERATE/SVML but not in MASSV?

1525 ↗

(On Diff #194193)

Maybe add a negative test where the nobuiltin attribute is present too.

[nemanjai]

@pjeeva01 Can you address Jinsong's comments? It would be good to add the negative tests for sure:

• No vectorization with -mattr=-altivec
• No vectorization with the nobuiltin attribute
• A couple of calls to functions that MASSV doesn't have a vector equivalent for

Also, it would be good to add a run line for big endian.

[pjeeva01]

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.

I have left the CHECK for *_massv function calls as-is because they are intended to check whether the massv suffixed calls are generated. The subsequent patch that further lowers such *_massv calls to PowerPC subtarget-specific entries will have checks with the full function signatures.

MASS vector library is currently supported only for 64-bit Linux LE. Since the use of this vector library is option controlled, I think it's fair for users to experience link errors if and when used with 32-bit Linux, for instance.

[jsji]

Some more nit-picker, other than that, mostly good to me.

However, as Chandler mentioned in similar patch for AArch64 in https://reviews.llvm.org/D53927,

In D53927#1307196, @chandlerc wrote:

Adding new support for a vector math library seems worth getting reviewed / approved by someone reasonably deeply familiar w/ our vectorization infra.

I would be good to have @hfinkel or @nemanjai or someone more familiar with vectorization infra to double confirm and approve it.

llvm/test/Transforms/LoopVectorize/PowerPC/massv-altivec.ll

2 ↗

(On Diff #198475)

I would be great if we can add some comments to describe the overall test points, eg: this is for negative test of massv without altivec support. This apply to all testcases in this patch.

llvm/test/Transforms/LoopVectorize/PowerPC/massv-calls.ll

282 ↗

(On Diff #194193)

Still missing in latest patch?

[pjeeva01]

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.

[nemanjai]

LGTM. Thanks for getting this done.

[nemanjai]

I'll commit this for you soon Jeeva.

[nemanjai]

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

[pjeeva01]

In D59881#1530258, @nemanjai wrote:

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

Thank you @nemanjai.

cfe/trunk/include/clang/Basic/CodeGenOptions.h

enum InliningMethod {
     NormalInlining,     // Use the standard function inlining pass.
     OnlyHintInlining,   // Inline only (implicitly) hinted functions.
     OnlyAlwaysInlining  // Only run the always inlining pass.
   };
 
   enum VectorLibrary {
     NoLibrary,  // Don't use any vector library.
     Accelerate, // Use the Accelerate framework.
+    MASSV,      // IBM MASS vector library.
     SVML        // Intel short vector math library.
   };
 
 
   enum ObjCDispatchMethodKind {
     Legacy = 0,
     NonLegacy = 1,
     Mixed = 2

cfe/trunk/include/clang/Driver/Options.td

 def fno_inline_functions : Flag<["-"], "fno-inline-functions">, Group<f_clang_Group>, Flags<[CC1Option]>;
 def fno_inline : Flag<["-"], "fno-inline">, Group<f_clang_Group>, Flags<[CC1Option]>;
 def fno_experimental_isel : Flag<["-"], "fno-experimental-isel">, Group<f_clang_Group>,
   HelpText<"Disables the experimental global instruction selector">;
 def fno_experimental_new_pass_manager : Flag<["-"], "fno-experimental-new-pass-manager">,
   Group<f_clang_Group>, Flags<[CC1Option]>,
   HelpText<"Disables an experimental new pass manager in LLVM.">;
 def fveclib : Joined<["-"], "fveclib=">, Group<f_Group>, Flags<[CC1Option]>,
-    HelpText<"Use the given vector functions library">, Values<"Accelerate,SVML,none">;
+    HelpText<"Use the given vector functions library">, Values<"Accelerate,MASSV,SVML,none">;
 def fno_lax_vector_conversions : Flag<["-"], "fno-lax-vector-conversions">, Group<f_Group>,
   HelpText<"Disallow implicit conversions between vectors with a different number of elements or different element types">, Flags<[CC1Option]>;
 def fno_merge_all_constants : Flag<["-"], "fno-merge-all-constants">, Group<f_Group>,
   HelpText<"Disallow merging of constants">;
 def fno_modules : Flag <["-"], "fno-modules">, Group<f_Group>,
   Flags<[DriverOption]>;
 def fno_implicit_module_maps : Flag <["-"], "fno-implicit-module-maps">, Group<f_Group>,
   Flags<[DriverOption]>;

cfe/trunk/lib/CodeGen/BackendUtil.cpp

for (auto &FuncName : CodeGenOpts.getNoBuiltinFuncs())
       if (TLII->getLibFunc(FuncName, F))
         TLII->setUnavailable(F);
   }
 
   switch (CodeGenOpts.getVecLib()) {
   case CodeGenOptions::Accelerate:
     TLII->addVectorizableFunctionsFromVecLib(TargetLibraryInfoImpl::Accelerate);
     break;
+  case CodeGenOptions::MASSV:
+    TLII->addVectorizableFunctionsFromVecLib(TargetLibraryInfoImpl::MASSV);
+    break;    
   case CodeGenOptions::SVML:
     TLII->addVectorizableFunctionsFromVecLib(TargetLibraryInfoImpl::SVML);
     break;
   default:
     break;
   }
   return TLII;
 }

cfe/trunk/lib/Frontend/CompilerInvocation.cpp

static bool ParseCodeGenArgs(CodeGenOptions &Opts, ArgList &Args, InputKind IK,
   Opts.DebugPassManager =
       Args.hasFlag(OPT_fdebug_pass_manager, OPT_fno_debug_pass_manager,
                    /* Default */ false);
 
   if (Arg *A = Args.getLastArg(OPT_fveclib)) {
     StringRef Name = A->getValue();
     if (Name == "Accelerate")
       Opts.setVecLib(CodeGenOptions::Accelerate);
+    else if (Name == "MASSV")
+      Opts.setVecLib(CodeGenOptions::MASSV);
     else if (Name == "SVML")
       Opts.setVecLib(CodeGenOptions::SVML);
     else if (Name == "none")
       Opts.setVecLib(CodeGenOptions::NoLibrary);
     else
       Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << Name;
   }
 

cfe/trunk/test/Driver/autocomplete.c

 // FFPALL: fast
 // FFPALL-NEXT: off
 // FFPALL-NEXT: on
 // RUN: %clang --autocomplete=-flto= | FileCheck %s -check-prefix=FLTOALL
 // FLTOALL: full
 // FLTOALL-NEXT: thin
 // RUN: %clang --autocomplete=-fveclib= | FileCheck %s -check-prefix=FVECLIBALL
 // FVECLIBALL: Accelerate
+// FVECLIBALL-NEXT: MASSV
 // FVECLIBALL-NEXT: none
 // FVECLIBALL-NEXT: SVML
 // RUN: %clang --autocomplete=-fshow-overloads= | FileCheck %s -check-prefix=FSOVERALL
 // FSOVERALL: all
 // FSOVERALL-NEXT: best
 // RUN: %clang --autocomplete=-fvisibility= | FileCheck %s -check-prefix=FVISIBILITYALL
 // FVISIBILITYALL: default
 // FVISIBILITYALL-NEXT: hidden

cfe/trunk/test/Driver/fveclib.c

 // RUN: %clang -### -c -fveclib=none %s 2>&1 | FileCheck -check-prefix CHECK-NOLIB %s
 // RUN: %clang -### -c -fveclib=Accelerate %s 2>&1 | FileCheck -check-prefix CHECK-ACCELERATE %s
+// RUN: %clang -### -c -fveclib=MASSV %s 2>&1 | FileCheck -check-prefix CHECK-MASSV %s
 // RUN: not %clang -c -fveclib=something %s 2>&1 | FileCheck -check-prefix CHECK-INVALID %s
 
 // CHECK-NOLIB: "-fveclib=none"
 // CHECK-ACCELERATE: "-fveclib=Accelerate"
+// CHECK-MASSV: "-fveclib=MASSV"
 
 // CHECK-INVALID: error: invalid value 'something' in '-fveclib=something'
 
 // RUN: %clang -fveclib=Accelerate %s -target arm64-apple-ios8.0.0 -### 2>&1 | FileCheck --check-prefix=CHECK-LINK %s
 // CHECK-LINK: "-framework" "Accelerate"
 
 // RUN: %clang -fveclib=Accelerate %s -nostdlib -target arm64-apple-ios8.0.0 -### 2>&1 | FileCheck --check-prefix=CHECK-LINK-NOSTDLIB %s
 // CHECK-LINK-NOSTDLIB-NOT: "-framework" "Accelerate"
 
 // RUN: %clang -fveclib=Accelerate %s -nodefaultlibs -target arm64-apple-ios8.0.0 -### 2>&1 | FileCheck --check-prefix=CHECK-LINK-NODEFAULTLIBS %s
 // CHECK-LINK-NODEFAULTLIBS-NOT: "-framework" "Accelerate"