On Sunday 05 July 2015 18:15:33 Francisco Jerez wrote:
> Hi EdB, a bunch of comments inline,
Hello
>
> EdB <edb+m...@sigluy.net> writes:
> > ---
> >
> > src/gallium/state_trackers/clover/api/program.cpp | 6 +-
> > .../state_trackers/clover/core/compiler.hpp | 7 +-
> > src/gallium/state_trackers/clover/core/error.hpp | 21 ++
> > src/gallium/state_trackers/clover/core/program.cpp | 93 ++++++-
> > src/gallium/state_trackers/clover/core/program.hpp | 10 +-
> > .../state_trackers/clover/llvm/invocation.cpp | 281
> > +++++++++++++++------ 6 files changed, 323 insertions(+), 95
> > deletions(-)
> >
> > diff --git a/src/gallium/state_trackers/clover/api/program.cpp
> > b/src/gallium/state_trackers/clover/api/program.cpp index
> > e9b1f38..2441d81 100644
> > --- a/src/gallium/state_trackers/clover/api/program.cpp
> > +++ b/src/gallium/state_trackers/clover/api/program.cpp
> > @@ -184,10 +184,6 @@ clBuildProgram(cl_program d_prog, cl_uint num_devs,
> >
> > prog.build(devs, opts);
>
> I don't think there's any reason to keep the program::build method
> around anymore, it's only going to be called from this entry point so
> you could as well make the two function calls to ::compile() and
>
> ::link() directly from here.
ok
> ::
> > return CL_SUCCESS;
> >
> > } catch (error &e) {
> >
> > - if (e.get() == CL_INVALID_COMPILER_OPTIONS)
> > - return CL_INVALID_BUILD_OPTIONS;
> > - if (e.get() == CL_COMPILE_PROGRAM_FAILURE)
> > - return CL_BUILD_PROGRAM_FAILURE;
> >
> > return e.get();
> >
> > }
> >
> > @@ -224,7 +220,7 @@ clCompileProgram(cl_program d_prog, cl_uint num_devs,
> >
> > range(header_names, num_headers),
> > objs<allow_empty_tag>(d_header_progs, num_headers));
> >
> > - prog.build(devs, opts, headers);
> > + prog.compile(devs, opts, headers);
> >
> > return CL_SUCCESS;
> >
> > } catch (error &e) {
> >
> > diff --git a/src/gallium/state_trackers/clover/core/compiler.hpp
> > b/src/gallium/state_trackers/clover/core/compiler.hpp index
> > c68aa39..31fb6ee 100644
> > --- a/src/gallium/state_trackers/clover/core/compiler.hpp
> > +++ b/src/gallium/state_trackers/clover/core/compiler.hpp
> > @@ -32,11 +32,16 @@ namespace clover {
> >
> > module compile_program_llvm(const std::string &source,
> >
> > const header_map &headers,
> >
> > - pipe_shader_ir ir,
> >
> > const std::string &target,
> > const std::string &opts,
> > std::string &r_log);
> >
> > + module link_program_llvm(const std::vector<module> &modules,
> > + enum pipe_shader_ir ir,
> > + const std::string &target,
> > + const std::string &opts,
> > + std::string &r_log);
> > +
> >
> > module compile_program_tgsi(const std::string &source);
> >
> > }
> >
> > diff --git a/src/gallium/state_trackers/clover/core/error.hpp
> > b/src/gallium/state_trackers/clover/core/error.hpp index 780b973..3c1bf90
> > 100644
> > --- a/src/gallium/state_trackers/clover/core/error.hpp
> > +++ b/src/gallium/state_trackers/clover/core/error.hpp
> > @@ -68,10 +68,31 @@ namespace clover {
> >
> > class build_error : public error {
> >
> > public:
> > build_error(const std::string &what = "") :
> > + error(CL_BUILD_PROGRAM_FAILURE, what) {
> > + }
> > + };
> > +
>
> This exception class now seems redundant -- With program::build() gone
> build is no longer a thing.
It's still needed by tgsi.
I plan to rework this part later to make it consistent with the way it's
handle in llvm/invocation but first off I wanted to be done with clLink :/
>
> > + class compile_error : public error {
> > + public:
> >
> > + compile_error(const std::string &what = "") :
> > error(CL_COMPILE_PROGRAM_FAILURE, what) {
> >
> > }
> >
> > };
> >
> > + class link_error : public error {
> > + public:
> > + link_error(const std::string &what = "") :
> > + error(CL_LINK_PROGRAM_FAILURE, what) {
> > + }
> > + };
> > +
> > + class link_option_error : public error {
> > + public:
> > + link_option_error(const std::string &what = "") :
> > + error(CL_INVALID_LINKER_OPTIONS , what) {
> > + }
> > + };
> > +
>
> I don't think you really need to special-case link_option_error against
> the less specific clover::error class?
clLinkProgram should not create a program if it failed to parse the given
options, I will use this class to handle this case. Other case should create
the said program.
That said, it could also have been created in later patch.
>
> > template<typename O>
> > class invalid_object_error;
> >
> > diff --git a/src/gallium/state_trackers/clover/core/program.cpp
> > b/src/gallium/state_trackers/clover/core/program.cpp index
> > 0d6cc40..21faf4e 100644
> > --- a/src/gallium/state_trackers/clover/core/program.cpp
> > +++ b/src/gallium/state_trackers/clover/core/program.cpp
> > @@ -40,15 +40,37 @@ program::program(clover::context &ctx,
> >
> > }
> >
> > void
> >
> > -program::build(const ref_vector<device> &devs, const char *opts,
> > - const header_map &headers) {
> > +program::build(const ref_vector<device> &devs, const char *opts) {
> > +
>
> Unnecessary whitespace.
ok
>
> > + if (has_source) {
> > + try {
> > + compile(devs, opts, {});
> > + if (!link(devs, opts, {*this}, true))
> > + throw error(CL_BUILD_PROGRAM_FAILURE);
> > + } catch (error &e) {
> > + switch (e.get()) {
> > + case CL_INVALID_COMPILER_OPTIONS:
> > + case CL_INVALID_LINKER_OPTIONS:
> > + e = error(CL_INVALID_BUILD_OPTIONS);
> > + break;
> > + case CL_COMPILE_PROGRAM_FAILURE:
> > + case CL_LINK_PROGRAM_FAILURE:
> > + e = error(CL_BUILD_PROGRAM_FAILURE);
> > + break;
>
> Ugh... This is mutating the caught exception through the l-value
> reference and rethrowing. Just saying 'throw error(...)' in each case
> statement would be cleaner. Doesn't matter anyway since this function
> is going away.
ok, I should have keep my first impression.
>
> > + }
> > + throw;
> > + }
> > + }
> > +}
> > +
> > +void
> > +program::compile(const ref_vector<device> &devs, const char *opts,
> > + const header_map &headers) {
> >
> > if (has_source) {
> >
> > _devices = devs;
> >
> > for (auto &dev : devs) {
> >
> > - _binaries.erase(&dev);
> > - _logs.erase(&dev);
> > - _opts.erase(&dev);
> > + clean(&dev);
> >
> > _opts.insert({ &dev, opts });
> >
> > @@ -58,12 +80,11 @@ program::build(const ref_vector<device> &devs, const
> > char *opts,>
> > auto module = (dev.ir_format() == PIPE_SHADER_IR_TGSI ?
> >
> > compile_program_tgsi(_source) :
> > compile_program_llvm(_source, headers,
> >
> > - dev.ir_format(),
> >
> > dev.ir_target(),
> > build_opts(dev),
> > log));
> >
> > _binaries.insert({ &dev, module });
> > _logs.insert({ &dev, log });
> >
> > - } catch (const build_error &) {
> > + } catch (const compile_error &) {
> >
> > _logs.insert({ &dev, log });
> > throw;
> >
> > }
> >
> > @@ -71,6 +92,64 @@ program::build(const ref_vector<device> &devs, const
> > char *opts,>
> > }
> >
> > }
> >
> > +bool
> > +program::link(const ref_vector<device> &devs, const char *opts,
> > + const ref_vector<program> &progs, bool keep_log) {
>
> I don't think that the keep_log option is very useful, link could just
> preserve the log in all cases -- That would work for clLinkProgram
> because there's no log in the first place, and for clBuildProgram too
> because the log has to be preserved. (And boolean flags that alter the
> semantics of a function are a rather dubious practice in general).
indeed
>
> > +
>
> Unnecessary whitespace.
ok
>
> > + bool r = true;
> > +
> > + _devices = devs;
> > +
> > + for (auto &dev : devs) {
> > + std::vector<module> mods;
> > + mods.reserve(progs.size());
> > + for (auto &prog : progs)
> > + mods.push_back(prog.binary(dev));
> > +
>
> const std::vector<module> mods = map([&](const program &prog) {
> return prog.binary(dev);
> }, progs);
>
> > + clean(&dev, keep_log);
> > +
> > + _opts.insert({ &dev, opts });
> > +
> > + if (mods.size() == 0) {
> > + append_to_log(&dev, "Nothing to link.");
> > + r = false;
> > + continue;
> > + }
> > +
>
> This seems redundant, it's the api layer's responsibility to catch cases
> in which zero program objects are being linked, and return
> CL_INVALID_VALUE.
First patch made more checking based on program binary type.
Anyway it doesn't need to be there for the moment.
>
> > + std::string log;
> > +
> > + try {
> > + auto module = link_program_llvm(mods,
> > + dev.ir_format(),
> > dev.ir_target(),
> > + opts, log);
> > + _binaries.insert({ &dev, module });
> > + append_to_log(&dev, log);
> > + } catch (const link_option_error &) {
> > + append_to_log(&dev, log);
> > + throw;
> > + } catch (const error &) {
> > + append_to_log(&dev, log);
> > + r = false;
>
> I suggest you just catch "const error &", update the error log and
> rethrow here, so you save a catch block and an error subclass.
As explain clLinkProgram doesn't behave the same way regarding error during
option parsing and after.
>
> > + }
> > + }
> > +
> > + return r;
>
> Ugh... This is mixing exception-based and return code-based error
> handling in the same function. Please just throw link_error if anything
> goes wrong (what will also simplify all of its users because they will
> no longer have to check the return code explicitly).
ok
>
> > +}
> > +
> > +void
> > +program::clean(const device *dev, bool keep_log) {
> > + _binaries.erase(dev);
> > + _opts.erase(dev);
> > + if (!keep_log)
> > + _logs.erase(dev);
> > +}
>
> This method seems rather ad-hoc and doesn't look like it helps abstract
> any knowledge from the callers, because they need to manipulate the maps
> directly, and the different handling of build logs forced you to add a
> boolean argument that changes the semantics of the function specifically
>
> for each use-case. IMO code like the following:
> | clean(dev, true);
>
> is less obvious than the two lines of equivalent code (e.g. a casual
> reader might wonder what is being set to true, question that cannot be
> answered by looking at the caller alone). I suggest you just open-code
> the function, it will be less LoC overall anyway.
ok
>
> > +
> > +void
> > +program::append_to_log(const device *dev, const std::string &log) {
> > + std::string &l = _logs[dev];
> > + l.empty() ? l = log : l + "\n" + log;
> > +}
> > +
>
> This seems wrong. And why is it even necessary? Couldn't you just do
> "_logs[&dev] += message;" for each caller? The build logs should
> already have been newline-terminated, and if they weren't in some case
> it's hiding a bug somewhere else.
ok
>
> > const std::string &
> > program::source() const {
> >
> > return _source;
> >
> > diff --git a/src/gallium/state_trackers/clover/core/program.hpp
> > b/src/gallium/state_trackers/clover/core/program.hpp index
> > 183145e..314979b 100644
> > --- a/src/gallium/state_trackers/clover/core/program.hpp
> > +++ b/src/gallium/state_trackers/clover/core/program.hpp
> > @@ -47,8 +47,11 @@ namespace clover {
> >
> > program &
> > operator=(const program &prog) = delete;
> >
> > - void build(const ref_vector<device> &devs, const char *opts,
> > - const header_map &headers = {});
> > + void build(const ref_vector<device> &devs, const char *opts);
> > + void compile(const ref_vector<device> &devs, const char *opts,
> > + const header_map &headers);
> > + bool link(const ref_vector<device> &devs, const char *opts,
> > + const ref_vector<program> &progs, bool keep_log = false);
>
> How about we make opts a "const std::string &" for these methods? It's
> what the LLVM interfacing code expects now that everything uses STL
> types consistently.
I was keeping the same signature, I will change that
>
> > const bool has_source;
> > const std::string &source() const;
> >
> > @@ -69,6 +72,9 @@ namespace clover {
> >
> > friend class kernel;
> >
> > private:
> > + void clean(const device *dev, bool keep_log = false);
> > + void append_to_log(const device *dev, const std::string &log);
> > +
> >
> > std::vector<intrusive_ref<device>> _devices;
> > std::map<const device *, module> _binaries;
> > std::map<const device *, std::string> _logs;
>
> I've stopped reviewing at this point because the changes below seem like
> a considerable amount of rather complex churn that could have been done
> independently from the changes to the core layer. Maybe split them as a
> separate patch?
I will made another version.
EdB
>
> > diff --git a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> > b/src/gallium/state_trackers/clover/llvm/invocation.cpp index
> > 9b91fee..ce5e3f2 100644
> > --- a/src/gallium/state_trackers/clover/llvm/invocation.cpp
> > +++ b/src/gallium/state_trackers/clover/llvm/invocation.cpp
> > @@ -108,7 +108,7 @@ namespace {
> >
> > name, llvm::MemoryBuffer::getMemBuffer(source));
> >
> > if (!c.ExecuteAction(act))
> >
> > - throw build_error(log);
> > + throw compile_error(log);
> >
> > }
> >
> > module
> >
> > @@ -130,22 +130,15 @@ namespace {
> >
> > }
> >
> > }
> >
> > - llvm::Module *
> > - compile_llvm(llvm::LLVMContext &llvm_ctx, const std::string &source,
> > - const header_map &headers,
> > - const std::string &name, const std::string &triple,
> > - const std::string &processor, const std::string &opts,
> > - clang::LangAS::Map& address_spaces, unsigned
> > &optimization_level, - std::string &r_log) {
> > + bool
> > + create_from_arg_llvm(clang::CompilerInstance &c,
> > + const std::string &target, const std::string
> > &opts, + llvm::raw_string_ostream &log) {
> >
> > - clang::CompilerInstance c;
> > - clang::EmitLLVMOnlyAction act(&llvm_ctx);
> > - std::string log;
> > - llvm::raw_string_ostream s_log(log);
> > - std::string libclc_path = LIBCLC_LIBEXECDIR + processor + "-"
> > - + triple + ".bc";
> > -
> > - // Parse the compiler options:
> > + // Parse the compiler options.
> > + // A file name should be present at the end
> > + // and must have the .cl extension in order for the
> > + // CompilerInvocation class to recognize it as an OpenCL source
> > file.>
> > std::vector<std::string> opts_array;
> > std::istringstream ss(opts);
> >
> > @@ -155,8 +148,6 @@ namespace {
> >
> > opts_array.push_back(opt);
> >
> > }
> >
> > - opts_array.push_back(name);
> > -
> >
> > std::vector<const char *> opts_carray;
> > for (unsigned i = 0; i < opts_array.size(); i++) {
> >
> > opts_carray.push_back(opts_array.at(i).c_str());
> >
> > @@ -171,15 +162,59 @@ namespace {
> >
> > DiagsBuffer = new clang::TextDiagnosticBuffer();
> >
> > clang::DiagnosticsEngine Diags(DiagID, &*DiagOpts, DiagsBuffer);
> >
> > - bool Success;
> >
> > + bool Success;
> >
> > Success =
> > clang::CompilerInvocation::CreateFromArgs(c.getInvocation(),
> >
> > opts_carray.data(),
> > opts_carray.data() +
> > opts_carray.size(),
> > Diags);
> >
> > - if (!Success) {
> > +
> > + if (Success) {
> > + const size_t processor_str_len = target.find_first_of("-");
> > + const std::string processor(target, 0, processor_str_len);
> > + const std::string triple(target.begin() + processor_str_len + 1,
> > + target.end());
> > +
> > + c.getLangOpts().NoBuiltin = true;
> > + c.getTargetOpts().Triple = triple;
> > + c.getTargetOpts().CPU = processor;
> > +
> > + // This is a workaround for a Clang bug which causes the number
> > + // of warnings and errors to be printed to stderr.
> > + // http://www.llvm.org/bugs/show_bug.cgi?id=19735
> > + c.getDiagnosticOpts().ShowCarets = false;
> > + c.getInvocation().setLangDefaults(c.getLangOpts(),
> > clang::IK_OpenCL, +
> > clang::LangStandard::lang_opencl11); + c.createDiagnostics(
> > + new clang::TextDiagnosticPrinter(
> > + log, &c.getDiagnosticOpts())
> > + );
> > +
> > +
> > c.setTarget(clang::TargetInfo::CreateTargetInfo(c.getDiagnostics(), +
> > c.getInvocation().TargetOpts));
> > + }
> > +
> > + return Success;
> > + }
> > +
> > + llvm::Module *
> > + compile_llvm(llvm::LLVMContext &llvm_ctx, const std::string &source,
> > + const header_map &headers,
> > + const std::string &target, const std::string &opts,
> > + clang::LangAS::Map& address_spaces, unsigned
> > &optimization_level, + std::string &r_log) {
> > +
> > + clang::CompilerInstance c;
> > + clang::EmitLLVMOnlyAction act(&llvm_ctx);
> > + std::string log;
> > + llvm::raw_string_ostream s_log(log);
> > + const std::string name = "compile.cl";
> > +
> > + if (!create_from_arg_llvm(c, target, opts + " " + name, s_log)) {
> > + r_log = log;
> >
> > throw error(CL_INVALID_COMPILER_OPTIONS);
> >
> > }
> >
> > +
> >
> > c.getFrontendOpts().ProgramAction = clang::frontend::EmitLLVMOnly;
> > c.getHeaderSearchOpts().UseBuiltinIncludes = true;
> > c.getHeaderSearchOpts().UseStandardSystemIncludes = true;
> >
> > @@ -196,22 +231,6 @@ namespace {
> >
> > // clc.h requires that this macro be defined:
> > c.getPreprocessorOpts().addMacroDef("cl_clang_storage_class_specifi
> > ers");
> >
> > -
> > - c.getLangOpts().NoBuiltin = true;
> > - c.getTargetOpts().Triple = triple;
> > - c.getTargetOpts().CPU = processor;
> > -
> > - // This is a workaround for a Clang bug which causes the number
> > - // of warnings and errors to be printed to stderr.
> > - // http://www.llvm.org/bugs/show_bug.cgi?id=19735
> > - c.getDiagnosticOpts().ShowCarets = false;
> > - c.getInvocation().setLangDefaults(c.getLangOpts(),
> > clang::IK_OpenCL,
> > -
> > clang::LangStandard::lang_opencl11); - c.createDiagnostics(
> > - new clang::TextDiagnosticPrinter(
> > - s_log,
> > - &c.getDiagnosticOpts()));
> > -
> >
> > #if HAVE_LLVM >= 0x0306
> >
> > c.getPreprocessorOpts().addRemappedFile(name,
> >
> > llvm::MemoryBuffer::getMemB
> > uffer(source).release());
> >
> > @@ -247,6 +266,7 @@ namespace {
> >
> > // attribute. This attribute will prevent Clang from creating
> > // illegal uses of barrier() (e.g. Moving barrier() inside a
> > conditional
> > // that is no executed by all threads) during its optimizaton
> > passes.
> >
> > + const std::string libclc_path = LIBCLC_LIBEXECDIR + target + ".bc";
> >
> > c.getCodeGenOpts().LinkBitcodeFile = libclc_path;
> >
> > optimization_level = c.getCodeGenOpts().OptimizationLevel;
> >
> > @@ -256,7 +276,7 @@ namespace {
> >
> > r_log = log;
> >
> > if (!ExecSuccess)
> >
> > - throw build_error();
> > + throw compile_error();
> >
> > // Get address spaces map to be able to find kernel argument
> > address space
> > memcpy(address_spaces, c.getTarget().getAddressSpaceMap(),
> >
> > @@ -485,7 +505,7 @@ namespace {
> >
> > LLVMDisposeMessage(err_message);
> >
> > if (err) {
> >
> > - throw build_error();
> > + throw link_error();
> >
> > }
> >
> > }
> >
> > @@ -505,7 +525,7 @@ namespace {
> >
> > if (LLVMGetTargetFromTriple(triple.c_str(), &target,
> > &error_message)) {
> >
> > r_log = std::string(error_message);
> > LLVMDisposeMessage(error_message);
> >
> > - throw build_error();
> > + throw link_error();
> >
> > }
> >
> > LLVMTargetMachineRef tm = LLVMCreateTargetMachine(
> >
> > @@ -514,7 +534,7 @@ namespace {
> >
> > if (!tm) {
> >
> > r_log = "Could not create TargetMachine: " + triple;
> >
> > - throw build_error();
> > + throw link_error();
> >
> > }
> >
> > if (dump_asm) {
> >
> > @@ -567,7 +587,7 @@ namespace {
> >
> > const char *name;
> > if (gelf_getshdr(section, &symtab_header) != &symtab_header)
> > {
> >
> > r_log = "Failed to read ELF section header.";
> >
> > - throw build_error();
> > + throw link_error();
> >
> > }
> > name = elf_strptr(elf, section_str_index,
> > symtab_header.sh_name);
> >
> > if (!strcmp(name, ".symtab")) {
> >
> > @@ -577,9 +597,9 @@ namespace {
> >
> > }
> > if (!symtab) {
> >
> > r_log = "Unable to find symbol table.";
> >
> > - throw build_error();
> > + throw link_error();
> >
> > }
> >
> > - } catch (build_error &e) {
> > + } catch (link_error &e) {
> >
> > elf_end(elf);
> > throw e;
> >
> > }
> >
> > @@ -640,6 +660,7 @@ namespace {
> >
> > return m;
> >
> > }
> >
> > + template<typename T>
> >
> > void
> > diagnostic_handler(const llvm::DiagnosticInfo &di, void *data) {
> >
> > if (di.getSeverity() == llvm::DS_Error) {
> >
> > @@ -650,7 +671,7 @@ namespace {
> >
> > stream.flush();
> > *(std::string*)data = message;
> >
> > - throw build_error();
> > + throw T();
> >
> > }
> >
> > }
> >
> > @@ -689,38 +710,28 @@ namespace {
> >
> > module
> > clover::compile_program_llvm(const std::string &source,
> >
> > - const header_map &headers,
> > - enum pipe_shader_ir ir,
> > - const std::string &target,
> > - const std::string &opts,
> > - std::string &r_log) {
> > + const header_map &headers,
> > + const std::string &target,
> > + const std::string &opts,
> > + std::string &r_log) {
> >
> > init_targets();
> >
> > - std::vector<llvm::Function *> kernels;
> > - size_t processor_str_len = std::string(target).find_first_of("-");
> > - std::string processor(target, 0, processor_str_len);
> > - std::string triple(target, processor_str_len + 1,
> > - target.size() - processor_str_len - 1);
> >
> > clang::LangAS::Map address_spaces;
> > llvm::LLVMContext llvm_ctx;
> > unsigned optimization_level;
> >
> > - llvm_ctx.setDiagnosticHandler(diagnostic_handler, &r_log);
> > + llvm_ctx.setDiagnosticHandler(diagnostic_handler<compile_error>,
> > &r_log);>
> > if (get_debug_flags() & DBG_CLC)
> >
> > - debug_log("// Build options: " + opts + '\n' + source, ".cl");
> > + debug_log("// options: " + opts + '\n' + source, ".cl");
> >
> > // 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_llvm(llvm_ctx, source, headers,
> > "input.cl",
> > - triple, processor, opts,
> > address_spaces, + llvm::Module *mod = compile_llvm(llvm_ctx, source,
> > headers,
> > + target, opts, address_spaces,
> >
> > optimization_level, r_log);
> >
> > - find_kernels(mod, kernels);
> > -
> > - optimize(mod, optimization_level, kernels);
> > -
> >
> > if (get_debug_flags() & DBG_LLVM) {
> >
> > std::string log;
> > llvm::raw_string_ostream s_log(log);
> >
> > @@ -729,25 +740,18 @@ clover::compile_program_llvm(const std::string
> > &source,>
> > debug_log(log, ".ll");
> >
> > }
> >
> > + //serialize for later use
> >
> > module m;
> >
> > - // Build the clover::module
> > - switch (ir) {
> > - case PIPE_SHADER_IR_TGSI:
> > - //XXX: Handle TGSI
> > - assert(0);
> > - m = module();
> > - break;
> > - case PIPE_SHADER_IR_LLVM:
> > - m = build_module_llvm(mod, kernels, address_spaces);
> > - break;
> > - case PIPE_SHADER_IR_NATIVE: {
> > - std::vector<char> code = compile_native(mod, triple, processor,
> > - get_debug_flags() &
> > DBG_ASM, - r_log);
> > - m = build_module_native(code, mod, kernels, address_spaces,
> > r_log); - break;
> > - }
> > - }
> > + llvm::SmallVector<char, 1024> llvm_bitcode;
> > + llvm::raw_svector_ostream bitcode_ostream(llvm_bitcode);
> > + llvm::BitstreamWriter writer(llvm_bitcode);
> > + llvm::WriteBitcodeToFile(mod, bitcode_ostream);
> > + bitcode_ostream.flush();
> > +
> > + std::vector<char> data(llvm_bitcode.begin(), llvm_bitcode.end());
> > + m.secs.push_back(module::section(0, module::section::text,
> > + data.size(), data));
> > +
> >
> > #if HAVE_LLVM >= 0x0306
> >
> > // LLVM 3.6 and newer, the user takes ownership of the module.
> > delete mod;
> >
> > @@ -755,3 +759,120 @@ clover::compile_program_llvm(const std::string
> > &source,>
> > return m;
> >
> > }
> >
> > +
> > +
> > +module
> > +clover::link_program_llvm(const std::vector<module> &modules,
> > + enum pipe_shader_ir ir,
> > + const std::string &target,
> > + const std::string &opts,
> > + std::string &r_log) {
> > +
> > + init_targets();
> > +
> > + std::string options = opts;
> > + bool create_library = false;
> > + size_t pos = options.find("-create-library");
> > + if (pos != std::string::npos) {
> > + create_library = true;
> > + options.erase(pos, 15);
> > + }
> > + options += " link.cl";
> > +
> > + llvm::LLVMContext llvm_ctx;
> > + llvm_ctx.setDiagnosticHandler(diagnostic_handler<link_error>, &r_log);
> > +
> > + std::string ci_log;
> > + llvm::raw_string_ostream rso(ci_log);
> > + clang::CompilerInstance c;
> > + if (!create_from_arg_llvm(c, target, options, rso)) {
> > + r_log = ci_log;
> > + throw link_option_error();
> > + }
> > +
> > + llvm::Module linked_mod("link", llvm_ctx);
> > + llvm::Linker linker(&linked_mod);
> > +
> > + for (const auto &mod : modules) {
> > + const auto s = llvm::StringRef(mod.secs[0].data.data(),
> > +
> > mod.secs[0].data.size()); +
> > + auto m =
> > + llvm::parseBitcodeFile(llvm::MemoryBufferRef(s, " "), llvm_ctx);
> > +
> > + if (!m) {
> > + r_log = m.getError().message();
> > + throw error(CL_INVALID_PROGRAM);
> > + }
> > +
> > +#if HAVE_LLVM < 0x0306
> > + if (linker.linkInModule(*m, &r_log))
> > +#elif HAVE_LLVM < 0x0307
> > + if (linker.linkInModule(*m))
> > +#else
> > + if (linker.linkInModule((*m).get()))
> > +#endif
> > + throw link_error();
> > + }
> > +
> > + module m;
> > + std::vector<llvm::Function *> kernels;
> > +
> > + //optimize
> > + if (!create_library)
> > + find_kernels(&linked_mod, kernels);
> > +
> > + unsigned optimization_level = c.getCodeGenOpts().OptimizationLevel;
> > + optimize(&linked_mod, optimization_level, kernels);
> > +
> > +
> > + unsigned debug_flags = get_debug_flags();
> > +
> > + if (debug_flags & DBG_LLVM) {
> > + std::string log;
> > + llvm::raw_string_ostream s_log(log);
> > + linked_mod.print(s_log, NULL);
> > + s_log.flush();
> > + debug_log(log, ".ll");
> > + }
> > +
> > + if (create_library) {
> > + //serialize for later use
> > + llvm::SmallVector<char, 1024> llvm_bitcode;
> > + llvm::raw_svector_ostream bitcode_ostream(llvm_bitcode);
> > + llvm::BitstreamWriter writer(llvm_bitcode);
> > + llvm::WriteBitcodeToFile(&linked_mod, bitcode_ostream);
> > + bitcode_ostream.flush();
> > +
> > + std::vector<char> data(llvm_bitcode.begin(), llvm_bitcode.end());
> > + m.secs.push_back(module::section(0, module::section::text,
> > + data.size(), data));
> > +
> > + } else {
> > + // Get address spaces map to be able to find kernel argument
> > address space + clang::LangAS::Map address_spaces;
> > + memcpy(address_spaces, c.getTarget().getAddressSpaceMap(),
> > +
> > sizeof(address_spaces)); +
> > + // Build the clover::module
> > + switch (ir) {
> > + case PIPE_SHADER_IR_TGSI:
> > + assert(0); // not supported
> > + break;
> > + case PIPE_SHADER_IR_LLVM:
> > + m = build_module_llvm(&linked_mod, kernels, address_spaces);
> > + break;
> > + case PIPE_SHADER_IR_NATIVE: {
> > + auto code = compile_native(&linked_mod,
> > c.getTargetOpts().Triple, +
> > c.getTargetOpts().CPU,
> > + debug_flags & DBG_ASM, r_log);
> > + m = build_module_native(code, &linked_mod, kernels,
> > address_spaces, + r_log);
> > + break;
> > + }
> > + }
> > +
> > + }
> > +
> > + return m;
> > +}
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev
