[OpenMP] TargetLibraryInfo from "declare simd".

[fpetrogalli]

Summary

This patch generates a list of global external variables that are
passed to the llvm::TargetLibraryInfo (TLI) to enable the vectorization of
loops containing calls to function that are marked with a #pragma omp
declare simd.

To do so, the global variables are generated as "global external", so
that they get removed in the middle-end and generated no useless code.

A new method of the TLI (provided in a separate LLVM patch) looks for
such global variables and updates the lists of vectorizable functions
that the vectorizer can use.

This behavior enables the programmer to provide vector routines that
are recognized as vectorizable by using the OpenMP directive "declare
simd" as follow:

$> clang -fopenmp -c -o whatever.o -O3 file.c

where file.c is

#pragma omp declare simd
double f(double x);

void aaa(double *x, double *y, int N) {
  for (int i = 0; i < N; ++i) {
    x[i] = f(y[i]);
  }
}

[fhahn]

lib/CodeGen/CGOpenMPRuntime.cpp

6521

Stray space

6522

Stray space

6615

Stray space in & CGM

6662

Missing space around / in Data.VecRegSize/ VLEN.getExtValue()

6676

Is the !! intentional?

6687

Is the !! intentional?

6739

Two spaces after =

[ABataev]

1. Please provide full context for your changes (check this document how to do this http://llvm.org/docs/Phabricator.html).
2. Use clang-format for all your changes.
3. Do not use backed infrastructure to mangle the name of the new function. Instead, you should build clang-based declaration and use clang infrastructure to ge

Also, I don't like the idea of the generation of real function profiles in the frontend. It was decided, that the frontend only provides a list of mangled names for vector functions, which must be generated in the backend. But you're trying something different. If you're going to change the whole solution you should send an RFC to the community

[fpetrogalli]

Rebase plus apply clang format.

[ABataev]

You did just some minor changes. The main questions are still unanswered.

[fpetrogalli]

I updated the comments related to code formatting.

[fpetrogalli]

In D27250#609006, @ABataev wrote:

Hi! Thanks for your review!

1. Please provide full context for your changes (check this document how to do this http://llvm.org/docs/Phabricator.html).

I missed that out, should be done now.

2. Use clang-format for all your changes.

Done.

3. Do not use backed infrastructure to mangle the name of the new function. Instead, you should build clang-based declaration and use clang infrastructure to ge

The second part of the sentence is missing. I guess you are saying I should use the infrastructure for name mangling in clang in llvm? Is that what you mean? I think that the right place to keep something that need to be shared between LLVM and clang is the LLVM codebase, not conversely. Can we wait for other people feedback before deciding here?

Also, I don't like the idea of the generation of real function profiles in the frontend. It was decided, that the frontend only provides a list of mangled names for vector functions, which must be generated in the backend.

I am just generating a set of global variable with external linkage that are deleted by LLVM before any code generation happens. The globals are used just to populate the TargetLibraryInfoImpl class instance created in BackendUtils.cpp. See the high level description of the functionality I have provided in the RFC mail (see [1] below).I see this mechanisms being easier rather than getting a name, de-mangle it, interpret the parameters section, and then generate whatever information the vectorizer need to vectorize a particular scalar function in a loop. Instead of introducing new structures for matching what the vectorizer need swith what the TLI provides, I just use what llvm already has, the FunctionType class.

This also avoid the complications of having to write a demangle function for the vector names for each architecture, which might have completely different names across architectures and ABI versions. With this FunctionType mechanisms, ABI changes are transparent to vectorizer.

But you're trying something different. If you're going to change the whole solution you should send an RFC to the community

I've sent out an email, apologies for not specifying RFC in the subject. You can find a copy of it with the RFC in the mailing list now [1].

[1] http://lists.llvm.org/pipermail/llvm-dev/2016-November/107677.html

[fpetrogalli]

In D27250#609063, @ABataev wrote:

You did just some minor changes. The main questions are still unanswered.

Hopefully now they are! :)

Francesco

[ABataev]

In D27250#609111, @fpetrogalli wrote:

In D27250#609006, @ABataev wrote:

Hi! Thanks for your review!

1. Please provide full context for your changes (check this document how to do this http://llvm.org/docs/Phabricator.html).

I missed that out, should be done now.

2. Use clang-format for all your changes.

Done.

3. Do not use backed infrastructure to mangle the name of the new function. Instead, you should build clang-based declaration and use clang infrastructure to ge

The second part of the sentence is missing. I guess you are saying I should use the infrastructure for name mangling in clang in llvm? Is that what you mean? I think that the right place to keep something that need to be shared between LLVM and clang is the LLVM codebase, not conversely. Can we wait for other people feedback before deciding here?

Yes, that's what I meant. Clang supports both Microsoft and Itanium-based manglings and in clang you should use clang-based solution.

Also, I don't like the idea of the generation of real function profiles in the frontend. It was decided, that the frontend only provides a list of mangled names for vector functions, which must be generated in the backend.

I am just generating a set of global variable with external linkage that are deleted by LLVM before any code generation happens. The globals are used just to populate the TargetLibraryInfoImpl class instance created in BackendUtils.cpp. See the high level description of the functionality I have provided in the RFC mail (see [1] below).I see this mechanisms being easier rather than getting a name, de-mangle it, interpret the parameters section, and then generate whatever information the vectorizer need to vectorize a particular scalar function in a loop. Instead of introducing new structures for matching what the vectorizer need swith what the TLI provides, I just use what llvm already has, the FunctionType class.

It does not matter. You should discuss it before trying to implement, especially if another solution was already accepted and implemented. Xinmin already answered you, please sync with him.

This also avoid the complications of having to write a demangle function for the vector names for each architecture, which might have completely different names across architectures and ABI versions. With this FunctionType mechanisms, ABI changes are transparent to vectorizer.

But you're trying something different. If you're going to change the whole solution you should send an RFC to the community

I've sent out an email, apologies for not specifying RFC in the subject. You can find a copy of it with the RFC in the mailing list now [1].

[1] http://lists.llvm.org/pipermail/llvm-dev/2016-November/107677.html

[fpetrogalli]

I have removed all reviewer, this is not intended for code review.

[fpetrogalli]

I have updated the name mangling of the associated vector function.

As requested in the RFC thread, I now use a C++ vector mangling that conforms to the approved C++ name mangling standard (on Itanium, I believe).

Vector function names are now in the form: ZVGQN2v_originalname, ZVGDN4v_originalname, where "D" and "Q" maps to NEON 64-bit registers and NEON 128-bits registers respectively.

I have added tests that show the full vectorization process, from C code to assembly. As I have already said, this algorithm is name mangling independent, which means that, even if I have changed the name mangling, there was no need to update the relative llvm patch: https://reviews.llvm.org/D27249

[fpetrogalli]

I have updated the patch to use the name mangling scheme we agreed - compatible with the GCC name mangling scheme.

We have choose letter "n" to identify the AdvancedSIMD vector extension of A64.

The code still interfaces with the TargetLibraryInfo: I understand this is not how the community has agreed to implement the "pragma omp declare simd functionality", but I am nevertheless updating this patch just to clarify the agreement around the name mangling of vector functions.

lib/CodeGen/BackendUtil.cpp

   std::unique_ptr<TargetLibraryInfoImpl> TLII(
       createTLII(TargetTriple, CodeGenOpts));
 
+  if (LangOpts.OpenMP) {
+    TLII->addOpenMPVectorFunctions(TheModule);
+  }
+
   switch (Inlining) {
   case CodeGenOptions::NoInlining:
     break;

lib/CodeGen/CGCall.cpp

 #include "CGBlocks.h"
 #include "CGCXXABI.h"
 #include "CGCleanup.h"
+#include "CGOpenMPRuntime.h"
 #include "CodeGenFunction.h"
 #include "CodeGenModule.h"
 #include "TargetInfo.h"

       const CXXMethodDecl *MD = dyn_cast<CXXMethodDecl>(Fn);
       if (Fn->isNoReturn() && !(AttrOnCallSite && MD && MD->isVirtual()))
         FuncAttrs.addAttribute(llvm::Attribute::NoReturn);
+
+      // TODO: this should work also when using -fopenmp-simd
+      if (getLangOpts().OpenMP && Fn->hasAttr<OMPDeclareSimdDeclAttr>()
+          && !Fn->hasBody()) {
+        auto MangledNames =
+          CGOpenMPRuntime(*this).listAvailableVectorSignatures(Fn);
+        for (auto &MangledName : MangledNames)
+          FuncAttrs.addAttribute(MangledName);
+      }
     }
 
     // 'const', 'pure' and 'noalias' attributed functions are also nounwind.

lib/CodeGen/CGOpenMPRuntime.h

   virtual void emitDeclareSimdFunction(const FunctionDecl *FD,
                                        llvm::Function *Fn);
 
+  /// Provides all the names of the vector variants associated to a
+  /// function \param FD marked with "declare simd'
+  virtual std::vector<std::string>
+  listAvailableVectorSignatures(const FunctionDecl *FD);
+
   /// Emit initialization for doacross loop nesting support.
   /// \param D Loop-based construct used in doacross nesting construct.
   virtual void emitDoacrossInit(CodeGenFunction &CGF,

lib/CodeGen/CGOpenMPRuntime.cpp

 #include "clang/AST/Decl.h"
 #include "clang/AST/StmtOpenMP.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/Bitcode/BitcodeReader.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/DerivedTypes.h"

   return C.getTypeSize(CDT);
 }
 
-static void
-emitX86DeclareSimdFunction(const FunctionDecl *FD, llvm::Function *Fn,
-                           const llvm::APSInt &VLENVal,
+static std::vector<std::string>
+emitX86DeclareSimdFunction(const FunctionDecl *FD,
+                            llvm::APSInt VLENVal,
                             ArrayRef<ParamAttrTy> ParamAttrs,
-                           OMPDeclareSimdDeclAttr::BranchStateTy State) {
+                           OMPDeclareSimdDeclAttr::BranchStateTy State,
+                           const StringRef MangledName) {
+  std::vector<std::string> OutVec;
   struct ISADataTy {
     char ISA;
     unsigned VecRegSize;

         if (!!ParamAttr.Alignment)
           Out << 'a' << ParamAttr.Alignment;
       }
-      Out << '_' << Fn->getName();
-      Fn->addFnAttr(Out.str());
+      Out << '_' << MangledName;
+      OutVec.push_back(Out.str());
     }
   }
+  return OutVec;
+}
+
+static llvm::VectorType * getAArch64MaskTy(const char ISA,
+                                           const unsigned LaneSizeInBits,
+                                           const unsigned VLEN,
+                                           llvm::LLVMContext &C) {
+
+  switch (LaneSizeInBits) {
+  case 64:
+    return  llvm::VectorType::get(llvm::Type::getInt64Ty(C),VLEN);
+  case 32:
+    return  llvm::VectorType::get(llvm::Type::getInt32Ty(C),VLEN);
+  case 16:
+    return  llvm::VectorType::get(llvm::Type::getInt16Ty(C),VLEN);
+  case 8:
+    return  llvm::VectorType::get(llvm::Type::getInt8Ty(C),VLEN);
+  default :
+    llvm_unreachable("Type is not supported");
+  }
+  return nullptr;
 }
 
+static void
+emitAArch64DeclareSimdFunction(CodeGenModule & CGM, const FunctionDecl *FD,
+                           const llvm::APSInt &UserVLEN,
+                           ArrayRef<ParamAttrTy> ParamAttrs,
+                           OMPDeclareSimdDeclAttr::BranchStateTy State,
+                           const StringRef MangledName) {
+  struct ISADataTy {
+    char ISA;
+    unsigned VecRegSize;
+  };
+  // make this depend on the size of
+  const ISADataTy ISAData[] = {
+    { 'Q', 128 }, // NEON 128
+    { 'D', 64 },  // NEON 64
+  };
+  std::vector<char> Masked;
+  switch (State) {
+  case OMPDeclareSimdDeclAttr::BS_Undefined:
+    Masked = {'M', 'N'};
+    break;
+  case OMPDeclareSimdDeclAttr::BS_Notinbranch:
+    Masked = {'N'};
+    break;
+  case OMPDeclareSimdDeclAttr::BS_Inbranch:
+    Masked = {'M'};
+    break;
+  }
+
+  for (auto Mask : Masked) {
+    for (auto Data : ISAData) {
+      std::string Buffer;
+      llvm::raw_string_ostream Out(Buffer);
+      Out << "_VA64" << Data.ISA << Mask;
+      std::vector<llvm::Type *> Args;
+      auto VLEN = UserVLEN;
+
+      // Compute VLEN if the user hasn't provided one in the pragma.
+      if (!UserVLEN) {
+        auto CDTSize = evaluateCDTSize(FD, ParamAttrs);
+        VLEN =  llvm::APSInt::getUnsigned(Data.VecRegSize / CDTSize);
+      }
+      // that's not a vector, skip
+      if (VLEN == 1)
+        continue;
+
+      // Add extra paramter if the function is Masked
+      if (Mask == 'M') {
+        auto MaskTy = getAArch64MaskTy(Data.ISA,
+                                       Data.VecRegSize/ VLEN.getExtValue(),
+                                       VLEN.getExtValue(), CGM.getLLVMContext());
+        Args.push_back(MaskTy);
+      }
+
+      Out << VLEN;
+      unsigned Pos = 0;
+      for (auto &ParamAttr : ParamAttrs) {
+        switch (ParamAttr.Kind){
+        case LinearWithVarStride:
+          Out << 's' << ParamAttr.StrideOrArg;
+          break;
+        case Linear:
+          Out << 'l';
+          if (!!ParamAttr.StrideOrArg)
+            Out << ParamAttr.StrideOrArg;
+          break;
+        case Uniform:
+          Out << 'u';
+          break;
+        case Vector:
+          Out << 'v';
+          break;
+        }
+
+        if (!!ParamAttr.Alignment)
+          Out << 'a' << ParamAttr.Alignment;
+
+        llvm::Type *Ty =
+          CGM.getTypes().ConvertType(FD->getParamDecl(Pos)->getOriginalType());
+        llvm::Type *VecTy = llvm::VectorType::get(Ty, VLEN.getExtValue());
+        switch (ParamAttr.Kind){
+        default:
+          Args.push_back(VecTy);
+          break;
+        case Uniform:
+          Args.push_back(Ty);
+          break;
+        }
+
+      ++Pos;
+      }
+      Out << '_' << MangledName;
+      std::string GlobalName =
+        llvm::TargetLibraryInfoImpl::mangle(Out.str(), MangledName);
+      llvm::Type * RetTy = CGM.getTypes().ConvertType(FD->getReturnType());
+      llvm::Type *VecRetTy = llvm::VectorType::get(RetTy, VLEN.getExtValue());
+      llvm::FunctionType * FTy= llvm::FunctionType::get(VecRetTy, Args, false);
+      CGM.getModule().getOrInsertGlobal(GlobalName, FTy);
+     }
+   }
+ }
+
 void CGOpenMPRuntime::emitDeclareSimdFunction(const FunctionDecl *FD,
                                               llvm::Function *Fn) {
+
+  auto FunList = listAvailableVectorSignatures(FD);
+  for (auto &VectorName: FunList) {
+    std::string name = VectorName;
+    Fn->addFnAttr(name);
+  }
+}
+
+std::vector<std::string>
+CGOpenMPRuntime::listAvailableVectorSignatures(const FunctionDecl *FD) {
   ASTContext &C = CGM.getContext();
   FD = FD->getCanonicalDecl();
   // Map params to their positions in function decl.

     ParamPositions.insert({P->getCanonicalDecl(), ParamPos});
     ++ParamPos;
   }
+  std::vector<std::string> OutVec;
+  std::string MangledName =  CGM.getMangledName(GlobalDecl(FD));
   for (auto *Attr : FD->specific_attrs<OMPDeclareSimdDeclAttr>()) {
     llvm::SmallVector<ParamAttrTy, 8> ParamAttrs(ParamPositions.size());
     // Mark uniform parameters.

       VLENVal = VLEN->EvaluateKnownConstInt(C);
     OMPDeclareSimdDeclAttr::BranchStateTy State = Attr->getBranchState();
     if (CGM.getTriple().getArch() == llvm::Triple::x86 ||
-        CGM.getTriple().getArch() == llvm::Triple::x86_64)
-      emitX86DeclareSimdFunction(FD, Fn, VLENVal, ParamAttrs, State);
+        CGM.getTriple().getArch() == llvm::Triple::x86_64) {
+      const auto Out =
+        emitX86DeclareSimdFunction(FD, VLENVal, ParamAttrs, State, MangledName);
+      for (auto &VecName : Out)
+        OutVec.push_back(VecName);
+    }
+    if (CGM.getTriple().getArch() == llvm::Triple::aarch64) {
+      emitAArch64DeclareSimdFunction(CGM, FD, VLENVal,
+                                     ParamAttrs, State,
+                                     MangledName);
+    }
   }
+  return OutVec;
 }
 
 namespace {

test/OpenMP/declare_simd_no_definition.c

+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c -emit-llvm %s -o - -femit-all-decls -verify| FileCheck %s
+// RUN: %clang_cc1 -verify -triple aarch64-linux-gnu -fopenmp -x c -emit-llvm %s -o - -femit-all-decls -verify| FileCheck %s --check-prefix=AARCH64
+// expected-no-diagnostics
+
+#pragma omp declare simd
+double f(double x);
+
+void aaa(double *x, double *y, int N) {
+  for (int i = 0; i < N; ++i) {
+    x[i] = f(y[i]);
+  }
+}
+
+#pragma omp declare simd notinbranch
+#pragma omp declare simd uniform(y)
+double xpow(double x, double y);
+
+
+void bbb(double *x, double *y, int N) {
+  for (int i = 0; i < N; ++i) {
+    x[i] = xpow(y[i], N);
+  }
+}
+
+
+// CHECK-LABEL: define void @aaa
+// CHECK-DAG: %call = call double @f(double %{{[0-9]+}}) #[[attr:[0-9]+]]
+// CHECK: attributes #[[attr]]
+// CHECK-DAG: _ZGVbM2v_f
+// CHECK-DAG: _ZGVbN2v_f
+// CHECK-DAG: _ZGVcM4v_f
+// CHECK-DAG: _ZGVcN4v_f
+// CHECK-DAG: _ZGVdM4v_f
+// CHECK-DAG: _ZGVdN4v_f
+// CHECK-DAG: _ZGVeM8v_f
+// CHECK-DAG: _ZGVeN8v_f
+
+
+// AARCH64-DAG: @vec_prefix__VA64QM2v_f_vec_midfix_f_vec_postfix = external global <2 x double> (<2 x i64>, <2 x double>)
+// AARCH64-DAG: @vec_prefix__VA64QN2v_f_vec_midfix_f_vec_postfix = external global <2 x double> (<2 x double>)
+
+// AARCH64-DAG: @vec_prefix__VA64QM2vu_xpow_vec_midfix_xpow_vec_postfix = external global <2 x double> (<2 x i64>, <2 x double>, double)
+// AARCH64-DAG: @vec_prefix__VA64QN2vu_xpow_vec_midfix_xpow_vec_postfix = external global <2 x double> (<2 x double>, double)
+// AARCH64-DAG: @vec_prefix__VA64QN2vv_xpow_vec_midfix_xpow_vec_postfix = external global <2 x double> (<2 x double>, <2 x double>)
+

test/OpenMP/declare_simd_no_definition.cpp

+// RUN: %clang_cc1 -verify -triple x86_64-apple-darwin10 -fopenmp -x c++ -emit-llvm %s -o - -femit-all-decls -verify| FileCheck %s
+// RUN: %clang_cc1 -verify -triple aarch64-linux-gnu -fopenmp -x c++ -emit-llvm %s -o - -femit-all-decls -verify| FileCheck %s --check-prefix=AARCH64
+// expected-no-diagnostics
+
+#pragma omp declare simd
+double f(double x);
+
+#pragma omp declare simd
+float f(float x);
+
+void aaa(double *x, double *y, int N) {
+  for (int i = 0; i < N; ++i) {
+    x[i] = f(y[i]);
+  }
+}
+
+void aaa(float *x, float *y, int N) {
+  for (int i = 0; i < N; ++i) {
+    x[i] = f(y[i]);
+  }
+}
+
+// CHECK-LABEL: define void @_Z3aaaPdS_i
+// CHECK-DAG: %call = call double @_Z1fd(double %{{[0-9]+}}) #[[attrD:[0-9]+]]
+
+// CHECK-LABEL: define void @_Z3aaaPfS_i
+// CHECK-DAG: %call = call float @_Z1ff(float %{{[0-9]+}}) #[[attrF:[0-9]+]]
+
+// CHECK-DAG: attributes #[[attrD]]
+// CHECK-DAG: _ZGVbM2v__Z1fd
+// CHECK-DAG: _ZGVbN2v__Z1fd
+// CHECK-DAG: _ZGVcM4v__Z1fd
+// CHECK-DAG: _ZGVcN4v__Z1fd
+// CHECK-DAG: _ZGVdM4v__Z1fd
+// CHECK-DAG: _ZGVdN4v__Z1fd
+// CHECK-DAG: _ZGVeM8v__Z1fd
+// CHECK-DAG: _ZGVeN8v__Z1fd
+
+// CHECK-DAG: attributes #[[attrF]]
+// CHECK-DAG: _ZGVbM4v__Z1ff
+// CHECK-DAG: _ZGVbN4v__Z1ff
+// CHECK-DAG: _ZGVcM8v__Z1ff
+// CHECK-DAG: _ZGVcN8v__Z1ff
+// CHECK-DAG: _ZGVdM8v__Z1ff
+// CHECK-DAG: _ZGVdN8v__Z1ff
+// CHECK-DAG: _ZGVeM16v__Z1ff
+// CHECK-DAG: _ZGVeN16v__Z1ff
+
+// AARCH64-DAG: @vec_prefix__VA64QM2v__Z1fd_vec_midfix__Z1fd_vec_postfix = external global <2 x double> (<2 x i64>, <2 x double>)
+// AARCH64-DAG: @vec_prefix__VA64QN2v__Z1fd_vec_midfix__Z1fd_vec_postfix = external global <2 x double> (<2 x double>)
+
+// AARCH64-DAG: @vec_prefix__VA64DM2v__Z1ff_vec_midfix__Z1ff_vec_postfix = external global <2 x float> (<2 x i32>, <2 x float>)
+// AARCH64-DAG: @vec_prefix__VA64QM4v__Z1ff_vec_midfix__Z1ff_vec_postfix = external global <4 x float> (<4 x i32>, <4 x float>)
+// AARCH64-DAG: @vec_prefix__VA64DN2v__Z1ff_vec_midfix__Z1ff_vec_postfix = external global <2 x float> (<2 x float>)
+// AARCH64-DAG: @vec_prefix__VA64QN4v__Z1ff_vec_midfix__Z1ff_vec_postfix = external global <4 x float> (<4 x float>)