[Beignet] [PATCH] Change clang system call to libclang api call.

Yang, Rong R rong.r.yang at intel.com
Tue May 21 21:32:16 PDT 2013



-----Original Message-----
From: beignet-bounces+rong.r.yang=intel.com at lists.freedesktop.org [mailto:beignet-bounces+rong.r.yang=intel.com at lists.freedesktop.org] On Behalf Of Zhigang Gong
Sent: Tuesday, May 21, 2013 4:41 PM
To: Yang, Rong R
Cc: beignet at lists.freedesktop.org
Subject: Re: [Beignet] [PATCH] Change clang system call to libclang api call.

I tested this patch with LLVM3.2, and it works fine. As to the clang/llvm 3.1, I haven't tested it, but I have some minor comments as below, please check it out.
Thanks. 

On Tue, May 21, 2013 at 12:45:56PM +0800, Yang Rong wrote:
> The original call clang command directly as frontend. The implement is not very flexible.
> I change to call libclang apis, now support both clang 3.1 and clang 3.2.
> Now still write the intermediate to the file, for code simply.
> Also fix llvm 3.1 build errors for my vector scalarize commit.
> 
> Signed-off-by: Yang Rong <rong.r.yang at intel.com>
> ---
>  CMake/FindLLVM.cmake                  |   27 ++++++++-
>  backend/src/CMakeLists.txt            |    1 +
>  backend/src/backend/program.cpp       |  103 ++++++++++++++++++++++++++-------
>  backend/src/llvm/llvm_gen_backend.cpp |    7 +--
>  backend/src/llvm/llvm_scalarize.cpp   |   10 ++--
>  5 files changed, 116 insertions(+), 32 deletions(-)
> 
> diff --git a/CMake/FindLLVM.cmake b/CMake/FindLLVM.cmake index 
> c06b8a4..b320639 100644
> --- a/CMake/FindLLVM.cmake
> +++ b/CMake/FindLLVM.cmake
> @@ -8,7 +8,7 @@
>  # LLVM_FOUND       - True if llvm found.
>  if (LLVM_INSTALL_DIR)
>    find_program(LLVM_CONFIG_EXECUTABLE NAMES llvm-config-32 llvm-config-3.2 llvm-config DOC "llvm-config executable" PATHS ${LLVM_INSTALL_DIR} NO_DEFAULT_PATH)
> -else (LLVM_INSTALL_DIR)                                                                
> +else (LLVM_INSTALL_DIR)
If we really want to support llvm 3.1, I think we need to add llvm-config-31 llvm-config-3.1 here. Right? And at the source root directory's CMakeLists.txt, we need to change the Find_Package(LLVM 3.2) to Find_Package(LLVM 3.1). Then any llvm version larger than 3.1 will satisfy the checking, otherwise, it fails with only llvm 3.1 installed.

Yes. The changes used to test llvm 3.1 is not include in this patch. If also need that patch, I will send it later.

>    find_program(LLVM_CONFIG_EXECUTABLE NAMES llvm-config-32 
> llvm-config-3.2 llvm-config DOC "llvm-config executable")  endif 
> (LLVM_INSTALL_DIR)
>  
> @@ -66,3 +66,28 @@ execute_process(
>    OUTPUT_VARIABLE LLVM_MODULE_LIBS
>    OUTPUT_STRIP_TRAILING_WHITESPACE
>  )
> +
> +macro(add_one_lib name)
> +  FIND_LIBRARY(CLANG_LIB
> +    NAMES ${name}
> +    PATHS ${LLVM_LIBRARY_DIR} )
> +  set(CLANG_LIBRARIES ${CLANG_LIBRARIES} ${CLANG_LIB})
> +	unset(CLANG_LIB CACHE)
> +endmacro()
> +
> +#Assume clang lib path same as llvm lib path
> +add_one_lib("clangFrontend")
> +add_one_lib("clangSerialization")
> +add_one_lib("clangDriver")
> +add_one_lib("clangCodeGen")
> +add_one_lib("clangSema")
> +add_one_lib("clangStaticAnalyzerFrontend")
> +add_one_lib("clangStaticAnalyzerCheckers")
> +add_one_lib("clangStaticAnalyzerCore")
> +add_one_lib("clangAnalysis")
> +add_one_lib("clangEdit")
> +add_one_lib("clangAST")
> +add_one_lib("clangParse")
> +add_one_lib("clangSema")
> +add_one_lib("clangLex")
> +add_one_lib("clangBasic")
> diff --git a/backend/src/CMakeLists.txt b/backend/src/CMakeLists.txt 
> index 183517a..a0fe198 100644
> --- a/backend/src/CMakeLists.txt
> +++ b/backend/src/CMakeLists.txt
> @@ -116,6 +116,7 @@ target_link_libraries(
>                        ${DRM_INTEL_LIBRARY}
>                        ${DRM_LIBRARY}
>                        ${OPENGL_LIBRARIES}
> +                      ${CLANG_LIBRARIES}
>                        ${LLVM_MODULE_LIBS}
>                        ${CMAKE_THREAD_LIBS_INIT}
>                        ${CMAKE_DL_LIBS}) diff --git 
> a/backend/src/backend/program.cpp b/backend/src/backend/program.cpp 
> index c46c681..6816a13 100644
> --- a/backend/src/backend/program.cpp
> +++ b/backend/src/backend/program.cpp
> @@ -46,6 +46,23 @@
>  #define LLVM_VERSION_MINOR 0
>  #endif /* !defined(LLVM_VERSION_MINOR) */
>  
> +#include <clang/CodeGen/CodeGenAction.h> #include 
> +<clang/Frontend/CompilerInstance.h>
> +#include <clang/Frontend/CompilerInvocation.h>
> +#if LLVM_VERSION_MINOR <= 1
> +#include <clang/Frontend/DiagnosticOptions.h>
> +#else
> +#include <clang/Basic/DiagnosticOptions.h> #endif  /* 
> +LLVM_VERSION_MINOR <= 1 */ #include 
> +<clang/Frontend/TextDiagnosticPrinter.h>
> +#include <clang/Basic/TargetInfo.h>
> +#include <clang/Basic/TargetOptions.h> #include 
> +<llvm/ADT/IntrusiveRefCntPtr.h> #include <llvm/ADT/OwningPtr.h> 
> +#include <llvm/Module.h> #include <llvm/Bitcode/ReaderWriter.h> 
> +#include <llvm/Support/raw_ostream.h>
> +
>  namespace gbe {
>  
>    Kernel::Kernel(const std::string &name) :
> @@ -104,6 +121,71 @@ namespace gbe {
>      GBE_SAFE_DELETE(program);
>    }
>  
> +  static void buildModuleFromSource(const char* input, const char* output) {
> +    // Arguments to pass to the clang frontend
> +    vector<const char *> args;
> +    args.push_back("-emit-llvm");
> +    args.push_back("-O3");
> +    args.push_back("-triple");
> +    args.push_back("nvptx");
> +    args.push_back(input);
> +
> +    // The compiler invocation needs a DiagnosticsEngine so it can 
> +report problems #if LLVM_VERSION_MINOR <= 1
> +    args.push_back("-triple");
> +    args.push_back("ptx32");
> +
> +    clang::TextDiagnosticPrinter *DiagClient =
> +                             new clang::TextDiagnosticPrinter(llvm::errs(), clang::DiagnosticOptions());
> +    llvm::IntrusiveRefCntPtr<clang::DiagnosticIDs> DiagID(new clang::DiagnosticIDs());
> +    clang::DiagnosticsEngine Diags(DiagID, DiagClient); #else
> +    args.push_back("-ffp-contract=off");
> +    args.push_back("-triple");
> +    args.push_back("nvptx");
> +
> +    llvm::IntrusiveRefCntPtr<clang::DiagnosticOptions> DiagOpts = new clang::DiagnosticOptions();
> +    clang::TextDiagnosticPrinter *DiagClient =
> +                             new clang::TextDiagnosticPrinter(llvm::errs(), &*DiagOpts);
> +    llvm::IntrusiveRefCntPtr<clang::DiagnosticIDs> DiagID(new clang::DiagnosticIDs());
> +    clang::DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagClient); 
> +#endif /* LLVM_VERSION_MINOR <= 1 */
> +
> +    // Create the compiler invocation
> +    llvm::OwningPtr<clang::CompilerInvocation> CI(new clang::CompilerInvocation);
> +    clang::CompilerInvocation::CreateFromArgs(*CI,
> +                                              &args[0],
> +                                              &args[0] + args.size(),
> +                                              Diags);
> +
> +    // Create the compiler instance
> +    clang::CompilerInstance Clang;
> +    Clang.setInvocation(CI.take());
> +    // Get ready to report problems
> +    Clang.createDiagnostics(args.size(), &args[0]);
> +    if (!Clang.hasDiagnostics())
> +      return;
> +
> +    // Set Language
> +    clang::LangOptions & lang_opts = Clang.getLangOpts();
> +    lang_opts.OpenCL = 1;
> +
> +    // Create an action and make the compiler instance carry it out
> +    llvm::OwningPtr<clang::CodeGenAction> Act(new clang::EmitLLVMOnlyAction());
> +    if (!Clang.ExecuteAction(*Act))
> +      return;
> +
> +    llvm::Module *module = Act->takeModule();
> +
> +    std::string ErrorInfo;
> +    llvm::raw_fd_ostream OS(output, ErrorInfo,llvm::raw_fd_ostream::F_Binary);
> +    //still write to temp file for code simply, otherwise need add another function.
> +    //because gbe_program_new_from_llvm also be used by cl_program_create_from_llvm, can't be removed
> +    //TODO: Pass module to llvmToGen, if use module, should return Act and use OwningPtr out of this funciton
> +    llvm::WriteBitcodeToFile(module, OS);
> +    OS.close();
> +  }
> +
>    extern std::string ocl_stdlib_str;
>    extern std::string ocl_common_defines_str;
>    static gbe_program programNewFromSource(const char *source, @@ 
> -124,26 +206,7 @@ namespace gbe {
>      fwrite(source, strlen(source), 1, clFile);
>      fclose(clFile);
>  
> -    // Now compile the code to llvm using clang
> -#if LLVM_VERSION_MINOR <= 1
> -    std::string compileCmd = "clang -x cl -fno-color-diagnostics -emit-llvm -O3 -ccc-host-triple ptx32 -c ";
> -#else
> -    std::string compileCmd = "clang -ffp-contract=off -emit-llvm -O3 -target nvptx -x cl -c ";
> -#endif /* LLVM_VERSION_MINOR <= 1 */
> -    compileCmd += clName;
> -    compileCmd += " ";
> -    if(options)
> -      compileCmd += options;
> -    compileCmd += " -o ";
> -    compileCmd += llName;
> -
> -    // Open a pipe and compile from here. Using Clang API instead is better
> -    FILE *pipe = popen(compileCmd.c_str(), "r");
> -    FATAL_IF (pipe == NULL, "Unable to run extern compilation command");
> -    char msg[256];
> -    while (fgets(msg, sizeof(msg), pipe))
> -      std::cout << msg;
> -    pclose(pipe);
> +    buildModuleFromSource(clName.c_str(), llName.c_str());
>      remove(clName.c_str());
>  
>      // Now build the program from llvm diff --git 
> a/backend/src/llvm/llvm_gen_backend.cpp 
> b/backend/src/llvm/llvm_gen_backend.cpp
> index deda687..d9a773a 100644
> --- a/backend/src/llvm/llvm_gen_backend.cpp
> +++ b/backend/src/llvm/llvm_gen_backend.cpp
> @@ -853,11 +853,8 @@ namespace gbe
>        // Insert a new register for each function argument  #if 
> LLVM_VERSION_MINOR <= 1
>        const AttrListPtr &PAL = F.getAttributes();
> -      uint32_t argID = 1; // Start at one actually
So the previous implementation assign arg ID starting from 1 for the version prior to 3.2, right?
This seems strange for me. I checked the code and found it was modified to start from 0 by Boqun for the LLVM version after 3.2 at commit 438c798.... Boqun, could you give some comments here?

Before Boqun's commit, it will build error with 3.1. Boqun set argID from 1 because class AttrListPtr's first attribute is return attribute, and argument from index 1.

> -      for (; I != E; ++I, ++argID) {
> -#else
> -      for (; I != E; ++I, ++argID) {
>  #endif /* LLVM_VERSION_MINOR <= 1 */
> +      for (; I != E; ++I, ++argID) {
>          const std::string &argName = I->getName().str();
>          Type *type = I->getType();
>  
> @@ -892,7 +889,7 @@ namespace gbe
>            PointerType *pointerType = dyn_cast<PointerType>(type);
>            // By value structure
>  #if LLVM_VERSION_MINOR <= 1
> -          if (PAL.paramHasAttr(argID, Attribute::ByVal)) {
> +          if (PAL.paramHasAttr(argID+1, Attribute::ByVal)) {
>  #else
>            if (I->hasByValAttr()) {
>  #endif /* LLVM_VERSION_MINOR <= 1 */
> diff --git a/backend/src/llvm/llvm_scalarize.cpp 
> b/backend/src/llvm/llvm_scalarize.cpp
> index f71401f..ef431f2 100644
> --- a/backend/src/llvm/llvm_scalarize.cpp
> +++ b/backend/src/llvm/llvm_scalarize.cpp
> @@ -71,7 +71,11 @@
>  #include "llvm/IntrinsicInst.h"
>  #include "llvm/Module.h"
>  #include "llvm/Pass.h"
> +#if LLVM_VERSION_MINOR <= 1
> +#include "llvm/Support/IRBuilder.h"
> +#else
>  #include "llvm/IRBuilder.h"
> +#endif /* LLVM_VERSION_MINOR <= 1 */
>  #include "llvm/Support/CallSite.h"
>  #include "llvm/Support/CFG.h"
>  #include "llvm/Support/raw_ostream.h"
> @@ -730,13 +734,7 @@ namespace gbe {
>  
>      Function::arg_iterator I = F.arg_begin(), E = F.arg_end();
>  
> -#if LLVM_VERSION_MINOR <= 1
> -    const AttrListPtr &PAL = F.getAttributes();
> -    uint32_t argID = 1; // Start at one actually
> -    for (; I != E; ++I, ++argID) {
> -#else
>      for (; I != E; ++I) {
> -#endif /* LLVM_VERSION_MINOR <= 1 */
>        Type *type = I->getType();
>  
>        if(type->isVectorTy())
> --
> 1.7.9.5
> 
> _______________________________________________
> Beignet mailing list
> Beignet at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/beignet
_______________________________________________
Beignet mailing list
Beignet at lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/beignet


More information about the Beignet mailing list