EdB <edb+m...@sigluy.net> writes:
> On Sunday 05 July 2015 18:15:33 Francisco Jerez wrote:
>>[...]
>> > --- 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 :/
>
The tgsi path could also throw compile_error AFAICT?
>>
>> > + 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.
>
Ah, fair enough.
>>
>> > 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
>>[...]
>>
>> > + 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.
>
Still I doubt that you need to handle them separately here, just update
the log and rethrow whatever you got, clLinkProgram can still give
link_error special treatment and return the created program despite the
failure for the application to be able to read back the error log.
>[...]
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
