Tom Stellard <thomas.stell...@amd.com> writes:
> On Mon, May 14, 2012 at 05:19:10PM +0200, Francisco Jerez wrote:
>> Tom Stellard <tstel...@gmail.com> writes:
>>
>> > On Sun, May 13, 2012 at 04:10:50PM +0200, Francisco Jerez wrote:
>> >> Tom Stellard <tstel...@gmail.com> writes:
>> >>[...]
>> >> > I guess this might be able to work, but I'm not really sure why doing
>> >> > it this way would be better than what is implemented in this patch.
>> >> >
>> >> I just think it would be useful by itself to have the r600 back-end able
>> >> to emit machine code in a specific object file format, and at the same
>> >> time, by doing so you avoid driver-specific code in the state tracker
>> >> that does one thing or another depending on the hardware it's running
>> >> on, which seems like a layering violation to me.
>> >
>> > State trackers already use caps to do different things depending on
>> > what hardware it's running. So, I'm not really sure why using
>> > PIPE_SHADER_CAP_PREFFERED_IR is any different.
>> >
>>
>> I'm aware, but the fact that we're already doing it elsewhere doesn't
>> mean it's a good practice if it can be avoided.
>>
>> If it turns out it can't be avoided, IMHO, any driver caps that affect
>> the state tracker behaviour should have a well-defined meaning
>> independent from the pipe driver that is running behind, i.e. a state
>> tracker should never have to do: "if (r600) { /* do something */ } else
>> { /* do something else */ }".
>>
>
> I agree with you here. Just so I'm clear, when you say you don't want
> "if (r600) { /* do something */ } else { /* do something else */ }"
> are you referring to this block of code:
>
> module
> clover::compile_program_llvm(const compat::string &source,
> const compat::string &target) {
>
> if (target == compat::string("TGSI")) {
> #if 0
> build_binary(source, target, "cl_input");
> module m = load_binary("cl_input.o");
> std::remove("cl_input.o");
> return m;
> #else
> return module();
> } else {
> return build_module_llvm(source, target, "cl_input");
> }
> #endif
> }
>
> If so, would it be better if this decision was based on the value of
> PIPE_SHADER_CAP_PREFFERED_IR?
>
Well yeah but I would rather not have that conditional at all, for the
reasons I've already mentioned.>[...] >> >> I'm fine with the CL state tracker giving out code in LLVM IR, but, if >> >> it does, I really think it makes no sense for it to even know it's >> >> running on r600. >> >> >> > Even though LLVM is a generic representation it's not always possible >> > to produce an LLVM bitcode program that is guaranteed to run on every >> > target, so if we use LLVM IR it will have to some knowledge about that >> > target on which it will executed. >> > >> >> Yes, what we are discussing is how to pass this information from the >> pipe driver to the compiler. If you don't want to fix clang, and you >> don't like my other proposal either, I'd consider separating the driver >> cap that determines the IR from the one that determines the target, >> until we have a generic way to pass the target description to the >> compiler. >> > > I agree that having separate caps for the IR and the target is a good > idea. This is what I was proposing in the previous discussion about > this http://lists.freedesktop.org/archives/mesa-dev/2012-April/020959.html > > Maybe we can revisit this discussion. What do you think about limiting > the values of PIPE_SHADER_CAP_PREFFERED_IR to {TGSI, LLVM, NATIVE} and > then adding a cap like PIPE_SHADER_CAP_LLVM_TARGET that returns the name > of the target (e.g. r600 for r600g) > I think that's definitely a better plan than having the state tracker take one path or another depending on the target. > -Tom >
pgpD6JsZD3C7Z.pgp
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
