[Mesa-dev] [PATCH 1/2] clover: Added missing address space checking of kernel parameters
Tom Stellard
tom at stellard.net
Tue Jul 30 07:51:50 PDT 2013
On Wed, Jul 24, 2013 at 09:29:49AM -0400, Jonathan Charest wrote:
> Here is an updated patch with no line wrapping and respecting 80-column limit (for my changes).
>
Hi,
I've pushed a modified version of this patch that uses global arguments
for constant buffers so it doesn't break r600g, and I've pushed your
r600g lds size fix too. Thanks!
-Tom
> ---
> .../state_trackers/clover/llvm/invocation.cpp | 34 +++++++++++++++-------
> 1 file changed, 23 insertions(+), 11 deletions(-)
>
> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> index dae61f7..3846a6e 100644
> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> @@ -26,6 +26,7 @@
> #include <clang/Frontend/TextDiagnosticBuffer.h>
> #include <clang/Frontend/TextDiagnosticPrinter.h>
> #include <clang/CodeGen/CodeGenAction.h>
> +#include <clang/Basic/TargetInfo.h>
> #include <llvm/Bitcode/BitstreamWriter.h>
> #include <llvm/Bitcode/ReaderWriter.h>
> #include <llvm/Linker.h>
> @@ -113,7 +114,7 @@ namespace {
> llvm::Module *
> compile(const std::string &source, const std::string &name,
> const std::string &triple, const std::string &processor,
> - const std::string &opts) {
> + const std::string &opts, clang::LangAS::Map& address_spaces) {
>
> clang::CompilerInstance c;
> clang::CompilerInvocation invocation;
> @@ -204,6 +205,10 @@ namespace {
> if (!c.ExecuteAction(act))
> throw build_error(log);
>
> + // Get address spaces map to be able to find kernel argument address space
> + memcpy(address_spaces, c.getTarget().getAddressSpaceMap(),
> + sizeof(address_spaces));
> +
> return act.takeModule();
> }
>
> @@ -282,7 +287,8 @@ namespace {
>
> module
> build_module_llvm(llvm::Module *mod,
> - const std::vector<llvm::Function *> &kernels) {
> + const std::vector<llvm::Function *> &kernels,
> + clang::LangAS::Map& address_spaces) {
>
> module m;
> struct pipe_llvm_program_header header;
> @@ -318,14 +324,18 @@ namespace {
> }
>
> if (arg_type->isPointerTy()) {
> - // XXX: Figure out LLVM->OpenCL address space mappings for each
> - // target. I think we need to ask clang what these are. For now,
> - // pretend everything is in the global address space.
> unsigned address_space = llvm::cast<llvm::PointerType>(arg_type)->getAddressSpace();
> - switch (address_space) {
> - default:
> - args.push_back(module::argument(module::argument::global, arg_size));
> - break;
> + if (address_space == address_spaces[clang::LangAS::opencl_local
> + - clang::LangAS::Offset]) {
> + args.push_back(module::argument(module::argument::local,
> + arg_size));
> + } else if (address_space == address_spaces[
> + clang::LangAS::opencl_constant - clang::LangAS::Offset]) {
> + args.push_back(module::argument(module::argument::constant,
> + arg_size));
> + } else {
> + args.push_back(module::argument(module::argument::global,
> + arg_size));
> }
> } else {
> args.push_back(module::argument(module::argument::scalar, arg_size));
> @@ -358,10 +368,12 @@ clover::compile_program_llvm(const compat::string &source,
> std::string processor(target.begin(), 0, processor_str_len);
> std::string triple(target.begin(), processor_str_len + 1,
> target.size() - processor_str_len - 1);
> + clang::LangAS::Map address_spaces;
>
> // The input file name must have the .cl extension in order for the
> // CompilerInvocation class to recognize it as an OpenCL source file.
> - llvm::Module *mod = compile(source, "input.cl", triple, processor, opts);
> + llvm::Module *mod = compile(source, "input.cl", triple, processor, opts,
> + address_spaces);
>
> find_kernels(mod, kernels);
>
> @@ -374,6 +386,6 @@ clover::compile_program_llvm(const compat::string &source,
> assert(0);
> return module();
> default:
> - return build_module_llvm(mod, kernels);
> + return build_module_llvm(mod, kernels, address_spaces);
> }
> }
> --
> 1.8.3.3
>
>
> On 2013-07-24 03:58, Francisco Jerez wrote:
> > Tom Stellard <tom at stellard.net> writes:
> >
> >> On Mon, Jul 22, 2013 at 09:24:12AM -0400, Jonathan Charest wrote:
> >>> To have non-static buffers in local memory, it is necessary to pass them
> >>> as arguments to the kernel. This was almost supported but the address
> >>> space mapping was missing to perform the check (thanks to tstellar for
> >>> pointing me in the right direction).
> >>>
> >>> ---
> >>> .../state_trackers/clover/llvm/invocation.cpp | 31
> >>> ++++++++++++++--------
> >>
> >> Reviewed-by: Tom Stellard <thomas.stellard at amd.com>
> >>
> > Looks OK to me, but the patch doesn't apply because of the line
> > wrapping. Also make sure you place block braces consistently and
> > respect the 80-column limit as far as possible.
> >
> >>> 1 file changed, 20 insertions(+), 11 deletions(-)
> >>>
> >>> diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> >>> b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> >>> index dae61f7..5cb5724 100644
> >>> --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> >>> +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> >>> @@ -26,6 +26,7 @@
> >>> #include <clang/Frontend/TextDiagnosticBuffer.h>
> >>> #include <clang/Frontend/TextDiagnosticPrinter.h>
> >>> #include <clang/CodeGen/CodeGenAction.h>
> >>> +#include <clang/Basic/TargetInfo.h>
> >>> #include <llvm/Bitcode/BitstreamWriter.h>
> >>> #include <llvm/Bitcode/ReaderWriter.h>
> >>> #include <llvm/Linker.h>
> >>> @@ -113,7 +114,7 @@ namespace {
> >>> llvm::Module *
> >>> compile(const std::string &source, const std::string &name,
> >>> const std::string &triple, const std::string &processor,
> >>> - const std::string &opts) {
> >>> + const std::string &opts, clang::LangAS::Map& address_spaces) {
> >>>
> >>> clang::CompilerInstance c;
> >>> clang::CompilerInvocation invocation;
> >>> @@ -204,6 +205,9 @@ namespace {
> >>> if (!c.ExecuteAction(act))
> >>> throw build_error(log);
> >>>
> >>> + // Get address spaces map to be able to find kernel argument
> >>> address spaces
> >>> + memcpy(address_spaces, c.getTarget().getAddressSpaceMap(),
> >>> sizeof(address_spaces));
> >>> +
> >>> return act.takeModule();
> >>> }
> >>>
> >>> @@ -282,7 +286,8 @@ namespace {
> >>>
> >>> module
> >>> build_module_llvm(llvm::Module *mod,
> >>> - const std::vector<llvm::Function *> &kernels) {
> >>> + const std::vector<llvm::Function *> &kernels,
> >>> + clang::LangAS::Map& address_spaces) {
> >>>
> >>> module m;
> >>> struct pipe_llvm_program_header header;
> >>> @@ -318,14 +323,17 @@ namespace {
> >>> }
> >>>
> >>> if (arg_type->isPointerTy()) {
> >>> - // XXX: Figure out LLVM->OpenCL address space mappings
> >>> for each
> >>> - // target. I think we need to ask clang what these are.
> >>> For now,
> >>> - // pretend everything is in the global address space.
> >>> unsigned address_space =
> >>> llvm::cast<llvm::PointerType>(arg_type)->getAddressSpace();
> >>> - switch (address_space) {
> >>> - default:
> >>> -
> >>> args.push_back(module::argument(module::argument::global, arg_size));
> >>> - break;
> >>> + if (address_space ==
> >>> + address_spaces[clang::LangAS::opencl_local -
> >>> clang::LangAS::Offset]) {
> >>> +
> >>> args.push_back(module::argument(module::argument::local, arg_size));
> >>> + }
> >>> + else if (address_space ==
> >>> + address_spaces[clang::LangAS::opencl_constant -
> >>> clang::LangAS::Offset]) {
> >>> +
> >>> args.push_back(module::argument(module::argument::constant, arg_size));
> >>> + }
> >>> + else {
> >>> +
> >>> args.push_back(module::argument(module::argument::global, arg_size));
> >>> }
> >>> } else {
> >>>
> >>> args.push_back(module::argument(module::argument::scalar, arg_size));
> >>> @@ -358,10 +366,11 @@ clover::compile_program_llvm(const compat::string
> >>> &source,
> >>> std::string processor(target.begin(), 0, processor_str_len);
> >>> std::string triple(target.begin(), processor_str_len + 1,
> >>> target.size() - processor_str_len - 1);
> >>> + clang::LangAS::Map address_spaces;
> >>>
> >>> // The input file name must have the .cl extension in order for the
> >>> // CompilerInvocation class to recognize it as an OpenCL source file.
> >>> - llvm::Module *mod = compile(source, "input.cl", triple, processor,
> >>> opts);
> >>> + llvm::Module *mod = compile(source, "input.cl", triple, processor,
> >>> opts, address_spaces);
> >>>
> >>> find_kernels(mod, kernels);
> >>>
> >>> @@ -374,6 +383,6 @@ clover::compile_program_llvm(const compat::string
> >>> &source,
> >>> assert(0);
> >>> return module();
> >>> default:
> >>> - return build_module_llvm(mod, kernels);
> >>> + return build_module_llvm(mod, kernels, address_spaces);
> >>> }
> >>> }
> >>> -- 1.8.3.2
> >>>
> >>> _______________________________________________
> >>> mesa-dev mailing list
> >>> mesa-dev at lists.freedesktop.org
> >>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list