Jan Vesely <jan.ves...@rutgers.edu> writes: > [SNIP] >> > >> > > I also don't like that this way there is no difference between >> > > explicit and implicit kernel arguments. On the other hand it's simple, >> > > and does not need additional per driver code. >> > > >> > Yeah... We definitely want to hide these from the user, as e.g. the >> > CL_KERNEL_NUM_ARGS param is required by the spec to return the number of >> > arguments provided by the user, and we don't want the user to set >> > implicit args, so it gets a bit messy. I think I like better your >> > original idea of passing them as launch_grid() arguments, even though >> > the grid offset and dimension parameters are somewhat artificial from a >> > the hardware's point of view. >> >> sorry to bug you some more with this. I tried one more thing before >> going back to the launch_grid parameters. this time it implements a >> parallel infrastructure for implicit arguments by creating artificial >> module arguments for uint and size_t (I don't think we need more for >> implicit arguments). >> >> I only added the work dimension argument but adding more should be easy. >> If you think that the launch_grid way is better, I'll stop experimenting >> as I ran out of ideas I wanted to try. > > ping > should I just resend using git instead of attachments?
Hi Jan, I'm sorry, I finally had a while to have a look into this. I've taken your series and tried to fix the couple of issues I wasn't very comfortable with, see the attached series. Does it look OK to you? Note that it's completely untested, maybe you could give it a run on your system? Thanks. > >> >> thanks, >> jan > > [SNIP] > > -- > Jan Vesely <jan.ves...@rutgers.edu>
From bb592403bf43f11aba2299c498599827f295a648 Mon Sep 17 00:00:00 2001
From: Francisco Jerez <curroje...@riseup.net>
Date: Wed, 8 Oct 2014 17:29:14 +0300
Subject: [PATCH 1/4] clover: Use unreachable() from util/macros.h instead of
assert(0).
---
src/gallium/state_trackers/clover/Makefile.am | 1 +
src/gallium/state_trackers/clover/core/device.cpp | 6 ++----
src/gallium/state_trackers/clover/core/object.hpp | 1 +
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/gallium/state_trackers/clover/Makefile.am b/src/gallium/state_trackers/clover/Makefile.am
index cb1e9c2..62c13fa 100644
--- a/src/gallium/state_trackers/clover/Makefile.am
+++ b/src/gallium/state_trackers/clover/Makefile.am
@@ -6,6 +6,7 @@ AM_CPPFLAGS = \
$(GALLIUM_PIPE_LOADER_DEFINES) \
-DPIPE_SEARCH_DIR=\"$(libdir)/gallium-pipe\" \
-I$(top_srcdir)/include \
+ -I$(top_srcdir)/src \
-I$(top_srcdir)/src/gallium/include \
-I$(top_srcdir)/src/gallium/drivers \
-I$(top_srcdir)/src/gallium/auxiliary \
diff --git a/src/gallium/state_trackers/clover/core/device.cpp b/src/gallium/state_trackers/clover/core/device.cpp
index 12c9584..688a7dd 100644
--- a/src/gallium/state_trackers/clover/core/device.cpp
+++ b/src/gallium/state_trackers/clover/core/device.cpp
@@ -70,8 +70,7 @@ device::type() const {
case PIPE_LOADER_DEVICE_PLATFORM:
return CL_DEVICE_TYPE_GPU;
default:
- assert(0);
- return 0;
+ unreachable("Unknown device type.");
}
}
@@ -84,8 +83,7 @@ device::vendor_id() const {
case PIPE_LOADER_DEVICE_PCI:
return ldev->u.pci.vendor_id;
default:
- assert(0);
- return 0;
+ unreachable("Unknown device type.");
}
}
diff --git a/src/gallium/state_trackers/clover/core/object.hpp b/src/gallium/state_trackers/clover/core/object.hpp
index 697565c..daad068 100644
--- a/src/gallium/state_trackers/clover/core/object.hpp
+++ b/src/gallium/state_trackers/clover/core/object.hpp
@@ -32,6 +32,7 @@
#include "core/error.hpp"
#include "core/property.hpp"
#include "api/dispatch.hpp"
+#include "util/macros.h"
///
/// Main namespace of the CL state tracker.
--
2.1.1
From 21f34572ed3fcccb086e3b88bdfb27d664a6b82f Mon Sep 17 00:00:00 2001
From: Francisco Jerez <curroje...@riseup.net>
Date: Wed, 8 Oct 2014 17:32:18 +0300
Subject: [PATCH 2/4] clover: Add semantic information to module::argument for
implicit parameter passing.
---
src/gallium/state_trackers/clover/core/module.hpp | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/gallium/state_trackers/clover/core/module.hpp b/src/gallium/state_trackers/clover/core/module.hpp
index 18a5bfb..268e3ba 100644
--- a/src/gallium/state_trackers/clover/core/module.hpp
+++ b/src/gallium/state_trackers/clover/core/module.hpp
@@ -68,27 +68,35 @@ namespace clover {
sign_ext
};
+ enum semantic {
+ general,
+ grid_dimension,
+ grid_offset
+ };
+
argument(enum type type, size_t size,
size_t target_size, size_t target_align,
- enum ext_type ext_type) :
+ enum ext_type ext_type,
+ enum semantic semantic = general) :
type(type), size(size),
target_size(target_size), target_align(target_align),
- ext_type(ext_type) { }
+ ext_type(ext_type), semantic(general) { }
argument(enum type type, size_t size) :
type(type), size(size),
target_size(size), target_align(1),
- ext_type(zero_ext) { }
+ ext_type(zero_ext), semantic(general) { }
argument() : type(scalar), size(0),
target_size(0), target_align(1),
- ext_type(zero_ext) { }
+ ext_type(zero_ext), semantic(general) { }
type type;
size_t size;
size_t target_size;
size_t target_align;
ext_type ext_type;
+ semantic semantic;
};
struct symbol {
--
2.1.1
From f49d26066c72aeb789be06e738037929c79ad312 Mon Sep 17 00:00:00 2001
From: Francisco Jerez <curroje...@riseup.net>
Date: Wed, 8 Oct 2014 17:39:35 +0300
Subject: [PATCH 3/4] clover: Pass execution dimensions and offset to the
kernel as implicit arguments.
---
src/gallium/state_trackers/clover/core/kernel.cpp | 88 +++++++++++++++++------
src/gallium/state_trackers/clover/core/kernel.hpp | 8 ++-
2 files changed, 71 insertions(+), 25 deletions(-)
diff --git a/src/gallium/state_trackers/clover/core/kernel.cpp b/src/gallium/state_trackers/clover/core/kernel.cpp
index e4b2152..7394586 100644
--- a/src/gallium/state_trackers/clover/core/kernel.cpp
+++ b/src/gallium/state_trackers/clover/core/kernel.cpp
@@ -33,24 +33,8 @@ kernel::kernel(clover::program &prog, const std::string &name,
program(prog), _name(name), exec(*this),
program_ref(prog._kernel_ref_counter) {
for (auto &marg : margs) {
- if (marg.type == module::argument::scalar)
- _args.emplace_back(new scalar_argument(marg.size));
- else if (marg.type == module::argument::global)
- _args.emplace_back(new global_argument);
- else if (marg.type == module::argument::local)
- _args.emplace_back(new local_argument);
- else if (marg.type == module::argument::constant)
- _args.emplace_back(new constant_argument);
- else if (marg.type == module::argument::image2d_rd ||
- marg.type == module::argument::image3d_rd)
- _args.emplace_back(new image_rd_argument);
- else if (marg.type == module::argument::image2d_wr ||
- marg.type == module::argument::image3d_wr)
- _args.emplace_back(new image_wr_argument);
- else if (marg.type == module::argument::sampler)
- _args.emplace_back(new sampler_argument);
- else
- throw error(CL_INVALID_KERNEL_DEFINITION);
+ if (marg.semantic == module::argument::general)
+ _args.emplace_back(argument::create(marg));
}
}
@@ -70,7 +54,7 @@ kernel::launch(command_queue &q,
const auto m = program().binary(q.device());
const auto reduced_grid_size =
map(divides(), grid_size, block_size);
- void *st = exec.bind(&q);
+ void *st = exec.bind(&q, grid_offset);
// The handles are created during exec_context::bind(), so we need make
// sure to call exec_context::bind() before retrieving them.
@@ -165,17 +149,39 @@ kernel::exec_context::~exec_context() {
}
void *
-kernel::exec_context::bind(intrusive_ptr<command_queue> _q) {
+kernel::exec_context::bind(intrusive_ptr<command_queue> _q,
+ const std::vector<size_t> &grid_offset) {
std::swap(q, _q);
// Bind kernel arguments.
auto &m = kern.program().binary(q->device());
auto margs = find(name_equals(kern.name()), m.syms).args;
auto msec = find(type_equals(module::section::text), m.secs);
+ auto explicit_arg = kern._args.begin();
+
+ for (auto &marg : margs) {
+ if (marg.semantic == module::argument::general) {
+ (*(explicit_arg++))->bind(*this, marg);
+
+ } else if (marg.semantic == module::argument::grid_dimension) {
+ const cl_uint dimension = grid_offset.size();
+ auto arg = argument::create(marg);
+
+ arg->set(sizeof(dimension), &dimension);
+ arg->bind(*this, marg);
- for_each([=](kernel::argument &karg, const module::argument &marg) {
- karg.bind(*this, marg);
- }, kern.args(), margs);
+ } else if (marg.semantic == module::argument::grid_offset) {
+ for (cl_uint x : pad_vector(*q, grid_offset, 1)) {
+ auto arg = argument::create(marg);
+
+ arg->set(sizeof(x), &x);
+ arg->bind(*this, marg);
+ }
+
+ } else {
+ unreachable("Unknown argument semantic.");
+ }
+ }
// Create a new compute state if anything changed.
if (!st || q != _q ||
@@ -283,6 +289,42 @@ namespace {
}
}
+std::unique_ptr<kernel::argument>
+kernel::argument::create(const module::argument &marg) {
+ if (marg.type == module::argument::scalar)
+ return std::unique_ptr<kernel::argument>(
+ new scalar_argument(marg.size));
+
+ else if (marg.type == module::argument::global)
+ return std::unique_ptr<kernel::argument>(
+ new global_argument);
+
+ else if (marg.type == module::argument::local)
+ return std::unique_ptr<kernel::argument>(
+ new local_argument);
+
+ else if (marg.type == module::argument::constant)
+ return std::unique_ptr<kernel::argument>(
+ new constant_argument);
+
+ else if (marg.type == module::argument::image2d_rd ||
+ marg.type == module::argument::image3d_rd)
+ return std::unique_ptr<kernel::argument>(
+ new image_rd_argument);
+
+ else if (marg.type == module::argument::image2d_wr ||
+ marg.type == module::argument::image3d_wr)
+ return std::unique_ptr<kernel::argument>(
+ new image_wr_argument);
+
+ else if (marg.type == module::argument::sampler)
+ return std::unique_ptr<kernel::argument>(
+ new sampler_argument);
+
+ else
+ throw error(CL_INVALID_KERNEL_DEFINITION);
+}
+
kernel::argument::argument() : _set(false) {
}
diff --git a/src/gallium/state_trackers/clover/core/kernel.hpp b/src/gallium/state_trackers/clover/core/kernel.hpp
index f9e2765..bf5998d 100644
--- a/src/gallium/state_trackers/clover/core/kernel.hpp
+++ b/src/gallium/state_trackers/clover/core/kernel.hpp
@@ -46,7 +46,8 @@ namespace clover {
exec_context &
operator=(const exec_context &) = delete;
- void *bind(intrusive_ptr<command_queue> _q);
+ void *bind(intrusive_ptr<command_queue> _q,
+ const std::vector<size_t> &grid_offset);
void unbind();
kernel &kern;
@@ -68,7 +69,8 @@ namespace clover {
public:
class argument {
public:
- argument();
+ static std::unique_ptr<argument>
+ create(const module::argument &marg);
argument(const argument &arg) = delete;
argument &
@@ -92,6 +94,8 @@ namespace clover {
virtual void unbind(exec_context &ctx) = 0;
protected:
+ argument();
+
bool _set;
};
--
2.1.1
From 3c1827ac246bbcf26bb805105c53829c79ecccba Mon Sep 17 00:00:00 2001
From: Jan Vesely <jan.ves...@rutgers.edu>
Date: Wed, 8 Oct 2014 17:43:01 +0300
Subject: [PATCH 4/4] clover: Append implicit arguments to the kernel argument
list.
[ Francisco Jerez: Split off from a larger patch, and take a slightly
different approach for passing the implicit arguments around. ]
Reviewed-by: Francisco Jerez <curroje...@riseup.net>
---
.../state_trackers/clover/llvm/invocation.cpp | 35 +++++++++++++++++-----
1 file changed, 27 insertions(+), 8 deletions(-)
diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
index 7bca0d6..45e8416 100644
--- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
+++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
@@ -308,6 +308,13 @@ namespace {
bitcode_ostream.flush();
for (unsigned i = 0; i < kernels.size(); ++i) {
+#if HAVE_LLVM < 0x0302
+ llvm::TargetData TD(kernel_func->getParent());
+#elif HAVE_LLVM < 0x0305
+ llvm::DataLayout TD(kernel_func->getParent()->getDataLayout());
+#else
+ llvm::DataLayout TD(mod);
+#endif
llvm::Function *kernel_func;
std::string kernel_name;
compat::vector<module::argument> args;
@@ -318,14 +325,6 @@ namespace {
for (llvm::Function::arg_iterator I = kernel_func->arg_begin(),
E = kernel_func->arg_end(); I != E; ++I) {
llvm::Argument &arg = *I;
-#if HAVE_LLVM < 0x0302
- llvm::TargetData TD(kernel_func->getParent());
-#elif HAVE_LLVM < 0x0305
- llvm::DataLayout TD(kernel_func->getParent()->getDataLayout());
-#else
- llvm::DataLayout TD(mod);
-#endif
-
llvm::Type *arg_type = arg.getType();
const unsigned arg_store_size = TD.getTypeStoreSize(arg_type);
@@ -384,6 +383,26 @@ namespace {
}
}
+ // 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.
+ llvm::Type *size_type =
+ TD.getSmallestLegalIntType(mod->getContext(), sizeof(cl_uint) * 8);
+
+ 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));
+
m.syms.push_back(module::symbol(kernel_name, 0, i, args ));
}
--
2.1.1
pgprOMyjfQNhs.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
