[Beignet] [PATCH 2/3] Add llvm instrinsic function llvm.memset and llvm.memcpy support.

[Zhigang Gong]

Some minor comments as below:

On Wed, Jan 15, 2014 at 04:31:05PM +0800, Yang Rong wrote:
> SPIR 1.2 require llvm.memcpy support. And llvm will emit llvm.memset sometimes.
> So adding a pass to lower these two intrinsic function, and then inline them.
> 
> In intrinsic lowering pass, find all llvm.memset and llvm.memcpy and then replace
> them with a function call __gen_memset_x and __gen_memcpy_xx, x and xx is for address space.
> 
> Because this pass is after clang, but after clang, the unused function seems be stripped, so
> implement the __gen_memset_x and __gen_memcpy_xx functions in pre compiled module, then link
> them.
> 
> Signed-off-by: Yang Rong <rong.r.yang at intel.com>
> ---
>  backend/src/CMakeLists.txt                   |   3 +-
>  backend/src/llvm/llvm_gen_backend.hpp        |   4 +
>  backend/src/llvm/llvm_intrinsic_lowering.cpp | 171 ++++++++++++++
>  backend/src/llvm/llvm_to_gen.cpp             |   2 +
>  backend/src/ocl_memcpy.ll                    | 323 +++++++++++++++++++++++++++
>  backend/src/ocl_memset.ll                    | 113 ++++++++++
>  6 files changed, 615 insertions(+), 1 deletion(-)
>  create mode 100644 backend/src/llvm/llvm_intrinsic_lowering.cpp
>  create mode 100644 backend/src/ocl_memcpy.ll
>  create mode 100644 backend/src/ocl_memset.ll
> 
> diff --git a/backend/src/CMakeLists.txt b/backend/src/CMakeLists.txt
> index b93133f..10bf67b 100644
> --- a/backend/src/CMakeLists.txt
> +++ b/backend/src/CMakeLists.txt
> @@ -136,6 +136,7 @@ else (GBE_USE_BLOB)
>      llvm/llvm_gen_backend.cpp
>      llvm/llvm_passes.cpp
>      llvm/llvm_scalarize.cpp
> +    llvm/llvm_intrinsic_lowering.cpp
>      llvm/llvm_to_gen.cpp
>      llvm/llvm_gen_backend.hpp
>      llvm/llvm_gen_ocl_function.hxx
> @@ -165,7 +166,7 @@ add_library (gbe STATIC ${GBE_SRC})
>  
>  # for pre compiled module library.
>  set (pcm_lib "beignet.bc")
> -set (pcm_sources ocl_barrier.ll)
> +set (pcm_sources ocl_barrier.ll ocl_memset.ll ocl_memcpy.ll)
>  ll_add_library (${pcm_lib} pcm_sources)
>  
>  ADD_DEPENDENCIES (gbe pch_object ${pcm_lib})
> diff --git a/backend/src/llvm/llvm_gen_backend.hpp b/backend/src/llvm/llvm_gen_backend.hpp
> index 55079f5..389d5f3 100644
> --- a/backend/src/llvm/llvm_gen_backend.hpp
> +++ b/backend/src/llvm/llvm_gen_backend.hpp
> @@ -84,8 +84,12 @@ namespace gbe
>    /*! Remove the GEP instructions */
>    llvm::BasicBlockPass *createRemoveGEPPass(const ir::Unit &unit);
>  
> +  /*! Scalarize all vector op instructions */
>    llvm::FunctionPass* createScalarizePass();
>  
> +  /*! Convert the Intrinsic call to gen function */
> +  llvm::BasicBlockPass *createIntrinsicLoweringPass();
> +
>  } /* namespace gbe */
>  
>  #endif /* __GBE_LLVM_GEN_BACKEND_HPP__ */
> diff --git a/backend/src/llvm/llvm_intrinsic_lowering.cpp b/backend/src/llvm/llvm_intrinsic_lowering.cpp
> new file mode 100644
> index 0000000..b6f874b
> --- /dev/null
> +++ b/backend/src/llvm/llvm_intrinsic_lowering.cpp
> @@ -0,0 +1,171 @@
> +/*
> + * Copyright © 2012 Intel Corporation
> + *
> + * This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU Lesser General Public
> + * License as published by the Free Software Foundation; either
> + * version 2 of the License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + * Lesser General Public License for more details.
> + *
> + * You should have received a copy of the GNU Lesser General Public
> + * License along with this library. If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +/**
> + * \file llvm_intrinisc_lowering.cpp
> + * \author Yang Rong <rong.r.yang at intel.com>
> + */
> +
> +#include "llvm/Config/config.h"
> +#if LLVM_VERSION_MINOR <= 2
> +#include "llvm/Function.h"
> +#include "llvm/InstrTypes.h"
> +#include "llvm/Instructions.h"
> +#include "llvm/IntrinsicInst.h"
> +#include "llvm/Module.h"
> +#else
> +#include "llvm/IR/Function.h"
> +#include "llvm/IR/InstrTypes.h"
> +#include "llvm/IR/Instructions.h"
> +#include "llvm/IR/IntrinsicInst.h"
> +#include "llvm/IR/Module.h"
> +#endif  /* LLVM_VERSION_MINOR <= 2 */
> +#include "llvm/Pass.h"
> +#if LLVM_VERSION_MINOR <= 1
> +#include "llvm/Support/IRBuilder.h"
> +#elif LLVM_VERSION_MINOR == 2
> +#include "llvm/IRBuilder.h"
> +#else
> +#include "llvm/IR/IRBuilder.h"
> +#endif /* LLVM_VERSION_MINOR <= 1 */
> +#include "llvm/Support/CallSite.h"
> +#include "llvm/Support/CFG.h"
> +#include "llvm/Support/raw_ostream.h"
> +
> +#include "llvm/llvm_gen_backend.hpp"
> +#include "sys/map.hpp"
> +
> +
> +using namespace llvm;
> +
> +namespace gbe {
> +    class InstrinsicLowering : public BasicBlockPass
> +    {
> +    public:
> +      static char ID;
> +      InstrinsicLowering() :
> +        BasicBlockPass(ID) {}
> +
> +      void getAnalysisUsage(AnalysisUsage &AU) const {
> +
> +      }
> +
> +      virtual const char *getPassName() const {
> +        return "PTX backend: lowering instrinsics";
We are now using spir backend.

> +      }
> +      static char convertSpaceToName(Value *val) {
> +        const uint32_t space = val->getType()->getPointerAddressSpace();
> +        switch(space) {
> +          case 0:
> +            return 'p';
> +          case 1:
> +            return 'g';
> +          case 3:
> +            return 'l';
> +          default:
> +            return '\0';

It's better to add an insert here to track invalid memory space.

The last comment is that it's better to add thos memcpy/memset.ll's
cl level source code. Maybe latter we need to make the cl->ll->bc
automatically.

All the other parts LGTM. Thanks.