[Mesa-dev] [PATCH 1/2] clover: Added missing address space checking of kernel parameters

[Francisco Jerez]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 229 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20130724/74381a94/attachment-0001.pgp>