Pierre Moreau <pierre.mor...@free.fr> writes:
> On 2018-01-23 — 14:04, Francisco Jerez wrote:
>> Pierre Moreau <pierre.mor...@free.fr> writes:
>>
>> > If creating a library, do not allow non-compiled object in it, as
>> > executables are not allowed, and libraries would make it really hard to
>> > enforce the "-enable-link-options" flag.
>> >
>> > Signed-off-by: Pierre Moreau <pierre.mor...@free.fr>
>> > ---
>> > src/gallium/state_trackers/clover/api/program.cpp | 19 ++++++++++++++++---
>> > 1 file changed, 16 insertions(+), 3 deletions(-)
>> >
>> > diff --git a/src/gallium/state_trackers/clover/api/program.cpp
>> > b/src/gallium/state_trackers/clover/api/program.cpp
>> > index 6044179587..8f0b103a4d 100644
>> > --- a/src/gallium/state_trackers/clover/api/program.cpp
>> > +++ b/src/gallium/state_trackers/clover/api/program.cpp
>> > @@ -251,9 +251,13 @@ clCompileProgram(cl_program d_prog, cl_uint num_devs,
>> > namespace {
>> > ref_vector<device>
>> > validate_link_devices(const ref_vector<program> &progs,
>> > - const ref_vector<device> &all_devs) {
>> > + const ref_vector<device> &all_devs,
>> > + const std::string &opts) {
>> > std::vector<device *> devs;
>> >
>> > + const std::string flag = "-create-library";
This seems to be unused.
>> > + const bool create_library = opts.find("-create-library") !=
>> > std::string::npos;
>> > +
>> > for (auto &dev : all_devs) {
>> > const auto has_binary = [&](const program &prog) {
>> > const auto t = prog.build(dev).binary_type();
>> > @@ -261,10 +265,19 @@ namespace {
>> > t == CL_PROGRAM_BINARY_TYPE_LIBRARY;
>> > };
>> >
>> > + // If creating a library, do not allow non-compiled object in
>> > it, as
>> > + // executables are not allowed, and libraries would make it
>> > really
>> > + // hard to enforce the "-enable-link-options".
>> > + if (create_library && any_of([&](const program &prog) {
>> > + const auto t = prog.build(dev).binary_type();
>> > + return t != CL_PROGRAM_BINARY_TYPE_COMPILED_OBJECT;
>> > + }, progs))
>> > + throw error(CL_INVALID_OPERATION);
>> > +
>>
>> Do you have any spec quote justifying this? "Hard" is hardly a
>> justification for emitting an unexpected API error ;). If doing such a
>> thing would rely on unimplemented behavior somewhere else it would
>> probably be cleaner to assert-fail wherever an implementation is missing
>> rather than returning an API error under conditions not expected by the
>> application.
>
> Re-reading the spec, I am quite sure that libraries can’t be made of
> libraries.
> From Section 5.6.5.1:
>
>> The following options can be specified when creating a library of compiled
>> binaries.
>>
>> -create-library
>> Create a library of compiled binaries specified in input_programs
>> argument to
>> clLinkProgram
>
Fair enough, can you add this sentence as a spec quote to the comment
above? With that taken into account:
Reviewed-by: Francisco Jerez <curroje...@riseup.net>
> and earlier on, in Section 5.6.3 when defining clLinkProgram, it makes the
> distinction between compiled binaries, libraries, and executables:
>
>> input_programs is an array of program objects that are compiled binaries or
>> libraries that are to be linked to create the program executable.
>
> (both extracts come from the 1.2 specification)
>
>>
>> > // According to the CL 1.2 spec, when "all programs specified
>> > [..]
>> > // contain a compiled binary or library for the device [..] a
>> > link is
>> > // performed",
>> > - if (all_of(has_binary, progs))
>> > + else if (all_of(has_binary, progs))
>> > devs.push_back(&dev);
>> >
>> > // otherwise if "none of the programs contain a compiled binary
>> > or
>> > @@ -290,7 +303,7 @@ clLinkProgram(cl_context d_ctx, cl_uint num_devs,
>> > const cl_device_id *d_devs,
>> > auto prog = create<program>(ctx);
>> > auto devs = validate_link_devices(progs,
>> > (d_devs ? objs(d_devs, num_devs) :
>> > - ref_vector<device>(ctx.devices())));
>> > + ref_vector<device>(ctx.devices())),
>> > opts);
>> >
>> > validate_build_common(prog, num_devs, d_devs, pfn_notify, user_data);
>> >
>> > --
>> > 2.16.0
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
