[Mesa-dev] [PATCH 20/47] clover/llvm: Clean up codestyle of get_kernel_args().
Jan Vesely
jan.vesely at rutgers.edu
Mon Jul 4 20:43:20 UTC 2016
On Mon, 2016-07-04 at 12:31 -0700, Francisco Jerez wrote:
> 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 ;)].
OK. FWIW, I tested the series and it works OK on my Turks setup (no
piglit regressions).
Jan
>
> > > +
> > > 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::ze
> > > ro_ext));
> > > - continue;
> > > - }
> > > -
> > > - // Image size implicit argument
> > > - if (type_name == "__llvm_image_size") {
> > > - args.push_back(module::argument(module::argument::sc
> > > alar,
> > > - sizeof(cl_uint),
> > > - TD.getTypeStoreSize(
> > > size_type),
> > > - TD.getABITypeAlignme
> > > nt(size_type),
> > > - module::argument::ze
> > > ro_ext,
> > > - module::argument::im
> > > age_size));
> > > - continue;
> > > - }
> > > -
> > > - // Image format implicit argument
> > > - if (type_name == "__llvm_image_format") {
> > > - args.push_back(module::argument(module::argument::sc
> > > alar,
> > > - sizeof(cl_uint),
> > > - TD.getTypeStoreSize(
> > > size_type),
> > > - TD.getABITypeAlignme
> > > nt(size_type),
> > > - module::argument::ze
> > > ro_ext,
> > > - module::argument::im
> > > age_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));
--
Jan Vesely <jan.vesely at rutgers.edu>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160704/7da7f3f7/attachment-0001.sig>
More information about the mesa-dev
mailing list