[Mesa-dev] [PATCH 1/2] clover: Added missing address space checking of kernel parameters
Jonathan Charest
jcharest+mesa-dev at gmail.com
Wed Jul 24 06:29:49 PDT 2013
Here is an updated patch with no line wrapping and respecting 80-column limit (for my changes).
---
.../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