[Mesa-dev] [PATCH 20/47] clover/llvm: Clean up codestyle of get_kernel_args().

Francisco Jerez currojerez at riseup.net
Mon Jul 4 19:31:04 UTC 2016


Jan Vesely <jan.vesely at rutgers.edu> writes:

> On Sun, 2016-07-03 at 17:51 -0700, Francisco Jerez wrote:
>> Reviewed-by: Serge Martin <edb+mesa at sigluy.net>
>> ---
>>  .../state_trackers/clover/llvm/invocation.cpp      | 223 ++++++++++-----------
>>  1 file changed, 103 insertions(+), 120 deletions(-)
>> 
>> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> index 754e477..0fc6190 100644
>> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
>> @@ -77,7 +77,10 @@
>>  using namespace clover;
>>  using namespace clover::llvm;
>>  
>> +using ::llvm::cast;
>> +using ::llvm::dyn_cast;
>>  using ::llvm::Function;
>> +using ::llvm::isa;
>>  using ::llvm::LLVMContext;
>>  using ::llvm::Module;
>>  using ::llvm::raw_string_ostream;
>> @@ -362,147 +365,127 @@ namespace {
>>     }
>>  #endif
>>  
>> +   enum module::argument::type
>> +   get_image_type(const std::string &type,
>> +                  const std::string &qual) {
>> +      if (type == "image2d_t" && qual == "read_only")
>> +         return module::argument::image2d_rd;
>> +      else if (type == "image2d_t" && qual == "write_only")
>> +         return module::argument::image2d_wr;
>> +      else if (type == "image3d_t" && qual == "read_only")
>> +         return module::argument::image3d_rd;
>> +      else if (type == "image3d_t" && qual == "write_only")
>> +         return module::argument::image3d_wr;
>> +      else
>> +         unreachable("Unknown image type");
>> +   }
>
> maybe add support for image1d_t?
>
It shouldn't be difficult but it will take an amount of boilerplate that
likely belongs in a different patch [for starters there are no
module::argument::image1d_* enums ;)].

>> +
>>     std::vector
>> -   get_kernel_args(const llvm::Module *mod, const std::string &kernel_name,
>> -                   const clang::CompilerInstance &c) {
>> +   make_kernel_args(const Module &mod, const std::string &kernel_name,
>> +                    const clang::CompilerInstance &c) {
>>        std::vector args;
>>        const auto address_spaces = c.getTarget().getAddressSpaceMap();
>> -      llvm::Function *kernel_func = mod->getFunction(kernel_name);
>> -      assert(kernel_func && "Kernel name not found in module.");
>> -      auto arg_md = get_kernel_arg_md(kernel_func);
>> -
>> -      llvm::DataLayout TD(mod);
>> -      llvm::Type *size_type =
>> -         TD.getSmallestLegalIntType(mod->getContext(), sizeof(cl_uint) * 8);
>> -
>> -      for (const auto &arg: kernel_func->args()) {
>> +      const Function &f = *mod.getFunction(kernel_name);
>> +      const auto arg_md = get_kernel_arg_md(&f);
>> +      ::llvm::DataLayout dl(&mod);
>> +      const auto size_type =
>> +         dl.getSmallestLegalIntType(mod.getContext(), sizeof(cl_uint) * 8);
>>  
>> -         llvm::Type *arg_type = arg.getType();
>> -         const unsigned arg_store_size = TD.getTypeStoreSize(arg_type);
>> +      for (const auto &arg : f.args()) {
>> +         const auto arg_type = arg.getType();
>>  
>>           // OpenCL 1.2 specification, Ch. 6.1.5: "A built-in data
>>           // type that is not a power of two bytes in size must be
>>           // aligned to the next larger power of two".  We need this
>>           // alignment for three element vectors, which have
>>           // non-power-of-2 store size.
>> +         const unsigned arg_store_size = dl.getTypeStoreSize(arg_type);
>>           const unsigned arg_api_size = util_next_power_of_two(arg_store_size);
>>  
>> -         llvm::Type *target_type = arg_type->isIntegerTy() ?
>> -               TD.getSmallestLegalIntType(mod->getContext(), arg_store_size * 8)
>> -               : arg_type;
>> -         unsigned target_size = TD.getTypeStoreSize(target_type);
>> -         unsigned target_align = TD.getABITypeAlignment(target_type);
>> -
>> -         llvm::StringRef type_name = arg_md[arg.getArgNo()].type_name;
>> -         llvm::StringRef access_qual = arg_md[arg.getArgNo()].access_qual;
>> -
>> -         // Image
>> -         const bool is_image2d = type_name == "image2d_t";
>> -         const bool is_image3d = type_name == "image3d_t";
>> -         if (is_image2d || is_image3d) {
>> -            const bool is_write_only = access_qual == "write_only";
>> -            const bool is_read_only = access_qual == "read_only";
>> -
>> -            enum module::argument::type marg_type;
>> -            if (is_image2d && is_read_only) {
>> -               marg_type = module::argument::image2d_rd;
>> -            } else if (is_image2d && is_write_only) {
>> -               marg_type = module::argument::image2d_wr;
>> -            } else if (is_image3d && is_read_only) {
>> -               marg_type = module::argument::image3d_rd;
>> -            } else if (is_image3d && is_write_only) {
>> -               marg_type = module::argument::image3d_wr;
>> -            } else {
>> -               assert(0 && "Wrong image access qualifier");
>> -            }
>> -
>> -            args.push_back(module::argument(marg_type,
>> -                                            arg_store_size, target_size,
>> -                                            target_align,
>> -                                            module::argument::zero_ext));
>> -            continue;
>> -         }
>> -
>> -         // Image size implicit argument
>> -         if (type_name == "__llvm_image_size") {
>> -            args.push_back(module::argument(module::argument::scalar,
>> -                                            sizeof(cl_uint),
>> -                                            TD.getTypeStoreSize(size_type),
>> -                                            TD.getABITypeAlignment(size_type),
>> -                                            module::argument::zero_ext,
>> -                                            module::argument::image_size));
>> -            continue;
>> -         }
>> -
>> -         // Image format implicit argument
>> -         if (type_name == "__llvm_image_format") {
>> -            args.push_back(module::argument(module::argument::scalar,
>> -                                            sizeof(cl_uint),
>> -                                            TD.getTypeStoreSize(size_type),
>> -                                            TD.getABITypeAlignment(size_type),
>> -                                            module::argument::zero_ext,
>> -                                            module::argument::image_format));
>> -            continue;
>> -         }
>> +         const auto target_type = !arg_type->isIntegerTy() ? arg_type :
>> +            dl.getSmallestLegalIntType(mod.getContext(), arg_store_size * 8);
>> +         const unsigned target_size = dl.getTypeStoreSize(target_type);
>> +         const unsigned target_align = dl.getABITypeAlignment(target_type);
>> +
>> +         const auto type_name = arg_md[arg.getArgNo()].type_name;
>> +
>> +         if (type_name == "image2d_t" || type_name == "image3d_t") {
>> +            // Image.
>> +            const auto access_qual = arg_md[arg.getArgNo()].access_qual;
>> +            args.emplace_back(get_image_type(type_name, access_qual),
>> +                              arg_store_size, target_size,
>> +                              target_align, module::argument::zero_ext);
>> +
>> +         } else if (type_name == "__llvm_image_size") {
>> +            // Image size implicit argument.
>> +            args.emplace_back(module::argument::scalar, sizeof(cl_uint),
>> +                              dl.getTypeStoreSize(size_type),
>> +                              dl.getABITypeAlignment(size_type),
>> +                              module::argument::zero_ext,
>> +                              module::argument::image_size);
>> +
>> +         } else if (type_name == "__llvm_image_format") {
>> +            // Image format implicit argument.
>> +            args.emplace_back(module::argument::scalar, sizeof(cl_uint),
>> +                              dl.getTypeStoreSize(size_type),
>> +                              dl.getABITypeAlignment(size_type),
>> +                              module::argument::zero_ext,
>> +                              module::argument::image_format);
>>  
>> -         // Other types
>> -         if (llvm::isa(arg_type) && arg.hasByValAttr()) {
>> -            arg_type =
>> -                  llvm::dyn_cast(arg_type)->getElementType();
>> -         }
>> +         } else {
>> +            // Other types.
>> +            const auto actual_type =
>> +               isa<::llvm::PointerType>(arg_type) && arg.hasByValAttr() ?
>> +               cast<::llvm::PointerType>(arg_type)->getElementType() : arg_type;
>> +
>> +            if (actual_type->isPointerTy()) {
>> +               const unsigned address_space =
>> +                  cast<::llvm::PointerType>(actual_type)->getAddressSpace();
>> +
>> +               if (address_space == address_spaces[clang::LangAS::opencl_local
>> +                                                   - clang::LangAS::Offset]) {
>> +                  args.emplace_back(module::argument::local, arg_api_size,
>> +                                    target_size, target_align,
>> +                                    module::argument::zero_ext);
>> +               } else {
>> +                  // XXX: Correctly handle constant address space.  There is no
>> +                  // way for r600g to pass a handle for constant buffers back
>> +                  // to clover like it can for global buffers, so
>> +                  // creating constant arguments will break r600g.  For now,
>> +                  // continue treating constant buffers as global buffers
>> +                  // until we can come up with a way to create handles for
>> +                  // constant buffers.
>> +                  args.emplace_back(module::argument::global, arg_api_size,
>> +                                    target_size, target_align,
>> +                                    module::argument::zero_ext);
>> +               }
>>  
>> -         if (arg_type->isPointerTy()) {
>> -            unsigned address_space = llvm::cast(arg_type)->getAddressSpace();
>> -            if (address_space == address_spaces[clang::LangAS::opencl_local
>> -                                                     - clang::LangAS::Offset]) {
>> -               args.push_back(module::argument(module::argument::local,
>> -                                               arg_api_size, target_size,
>> -                                               target_align,
>> -                                               module::argument::zero_ext));
>>              } else {
>> -               // XXX: Correctly handle constant address space.  There is no
>> -               // way for r600g to pass a handle for constant buffers back
>> -               // to clover like it can for global buffers, so
>> -               // creating constant arguments will break r600g.  For now,
>> -               // continue treating constant buffers as global buffers
>> -               // until we can come up with a way to create handles for
>> -               // constant buffers.
>> -               args.push_back(module::argument(module::argument::global,
>> -                                               arg_api_size, target_size,
>> -                                               target_align,
>> -                                               module::argument::zero_ext));
>> -           }
>> +               const bool needs_sign_ext = f.getAttributes().hasAttribute(
>> +                  arg.getArgNo() + 1, ::llvm::Attribute::SExt);
>>  
>> -         } else {
>> -            llvm::AttributeSet attrs = kernel_func->getAttributes();
>> -            enum module::argument::ext_type ext_type =
>> -                  (attrs.hasAttribute(arg.getArgNo() + 1,
>> -                                     llvm::Attribute::SExt) ?
>> -                   module::argument::sign_ext :
>> -                   module::argument::zero_ext);
>> -
>> -            args.push_back(
>> -               module::argument(module::argument::scalar, arg_api_size,
>> -                                target_size, target_align, ext_type));
>> +               args.emplace_back(module::argument::scalar, arg_api_size,
>> +                                 target_size, target_align,
>> +                                 (needs_sign_ext ? module::argument::sign_ext :
>> +                                  module::argument::zero_ext));
>> +            }
>>           }
>>        }
>>  
>>        // Append implicit arguments.  XXX - The types, ordering and
>>        // vector size of the implicit arguments should depend on the
>>        // target according to the selected calling convention.
>> -      args.push_back(
>> -         module::argument(module::argument::scalar, sizeof(cl_uint),
>> -                          TD.getTypeStoreSize(size_type),
>> -                          TD.getABITypeAlignment(size_type),
>> -                          module::argument::zero_ext,
>> -                          module::argument::grid_dimension));
>> -
>> -      args.push_back(
>> -         module::argument(module::argument::scalar, sizeof(cl_uint),
>> -                          TD.getTypeStoreSize(size_type),
>> -                          TD.getABITypeAlignment(size_type),
>> -                          module::argument::zero_ext,
>> -                          module::argument::grid_offset));
>> +      args.emplace_back(module::argument::scalar, sizeof(cl_uint),
>> +                        dl.getTypeStoreSize(size_type),
>> +                        dl.getABITypeAlignment(size_type),
>> +                        module::argument::zero_ext,
>> +                        module::argument::grid_dimension);
>> +
>> +      args.emplace_back(module::argument::scalar, sizeof(cl_uint),
>> +                        dl.getTypeStoreSize(size_type),
>> +                        dl.getABITypeAlignment(size_type),
>> +                        module::argument::zero_ext,
>> +                        module::argument::grid_offset);
>>  
>>        return args;
>>     }
>> @@ -531,7 +514,7 @@ namespace {
>>                                    find_kernels(const_cast(&mod)))) {
>>           if (offsets.count(name))
>>              m.syms.emplace_back(name, 0, offsets.at(name),
>> -                                get_kernel_args(&mod, name, c));
>> +                                make_kernel_args(mod, name, c));
>>        }
>>  
>>        m.secs.push_back(make_text_section(code));
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160704/88ee8a07/attachment-0001.sig>


More information about the mesa-dev mailing list