[Nouveau] Dealing with opencl kernel parameters in nouveau now that RES support is gone

[Hans de Goede]

Hi,

On 18-02-16 17:39, Ilia Mirkin wrote:
> On Thu, Feb 18, 2016 at 9:45 AM, Hans de Goede <hdegoede at redhat.com> wrote:
>> But this does not seem to be hooked up yet for nouveau.
>
> Samuel has patches. See
> https://cgit.freedesktop.org/~hakzsam/mesa/log/?h=arb_compute_shader_v3

Cool, I will take a look at those.

>> So some questions:
>> -The commit by Samual says:
>>   This introduces TGSI_FILE_MEMORY for shared, global and local memory. Only
>> shared memory is currently supported.
>>
>>   The commit introduces MEMORY[x] and MEMORY[x],SHARED so in reality it also
>> introduces
>>   a second option next to shared, so what are we going to use plain MEMORY[x]
>> for?
>>   I suggest using it for global memory but we need to be in agreement on
>> this.
>
> That sounds fine to me. However what I had in mind was switching the
> SHARED field into a 2-bit field and making it
>
> 1 = SHARED
> 2 = GLOBAL
> 3 = LOCAL
>
> (since for OpenCL you also need to be able to address local or private
> memory). I sorta wanted Samuel to do it, but since I had no idea where
> you were at, or if you were even still working on this, I figured it
> should be fixed up by the first person who needed it.

Sounds good, only the naming is somewhat unfortunate since opencl uses different
naming. I.e. it has no "shared"

OpenCL has:
-global:  accessible by all worker-groups as well as by the CPU
-const:   read-only global
-local:   shared by worker-items in the same worker-group, not shared between worker-groups
-private: accessible only to a single worker-item

So how do these map to the TGSI:

 > 1 = SHARED
 > 2 = GLOBAL
 > 3 = LOCAL

?


>
>>
>> -What about kernel input parameters, so far these have been using RES[32764]
>>   I must admit that I do not understand where the file_index of 32764 comes
>>   from (or where any of the file indexes come from for
>> src/gallium/tests/trivial/compute.c ?
>>   I have the feeling that these are not used at all, and everything simply
>> goes
>>   to a flat (virtual) memory space, with the params at address 0, correct ?
>
> It was never particularly well-specified, which was one of the reasons
> it went away. It also didn't map nicely onto the OpenGL model. There
> is a remaining question of how to do addressing in memory... there's
> 40 bits of address space. Should these implicitly be U64
> (dual-component in TGSI) addresses that are passed around? Not sure
> what the OpenCL position on all this is.

So far I've been using U32 for addresses as that is what Francisco's original
code was using. And this also is what things like the tgsi LOAD instruction
take. If you're doing a LOAD on a 1d buffer then you will use TEMP[#].x to
specify the index, and the way how this currently works with OpenCL is that
clCreateBuffer() will return a cl_mem type which then gets passed into
the kernel as input parameter and gets treated as a pointer by the compiler,
so e.g. global mem gets treated as a single address space even if there
are multiple global buffers and TEMP[#].x contains the value passed in
via cl_mem as start offset for the buffer + the index into the buffer.

So this means that currently we are limited to U32 since TEMP[#].x is
only 32 bits wide. Internally 40 bits addresses can and should probably
be used so that at least the different memory spaces each have the full
32 bits available.

Note that we could fix this by adding some sort of LOAD64 opcode, which
uses TEMP[#].x and TEMP[#].y as address for 1d buffers, I'm not sure
how this would work for 3d buffers though. I foresee the llvm backend
eventually getting a 64 bit mode where it will use 64 bits for all
pointers and use something like a LOAD64 opcode to indicate that
the indexes (which it effectively uses as addresses / pointers)
are 64 bit wide.

> Also it would be highly preferable to avoid using GLOBAL at all in the
> first place, and just use BUFFER[] or IMAGE[] or whatever. However
> after having a look at how OpenCL works, I don't think that's
> possible. But perhaps I missed something?

OpenCL is very C-like and as such uses a flat address space per memory
space, getting around that is going to be very tricky, if possible at
all.

>>   This seems like a nvidia implementation detail though, and we should IMHO
>> still
>>   implement a specific TGSI register file for accessing kernel input params,
>>   agreed ?
>
> I believe that kernel input params should be passed in via user
> uniforms. i.e. CONST. It doesn't have to be a separate UBO, it can use
> the user buffer thing, which is basically what it is now.
>
>>
>> If we can agree on the TGSI syntax for all this, then I'm pretty sure I can
>> hack
>> something together to get things to work with the latest master again, and
>> then we
>> can use this as a starting point for properly implementing this.
>>
>> Regards,
>>
>> Hans
>>
>>
>> p.s.
>>
>> I must say I was somewhat unpleasantly surprised by the RES breakage, yes I
>> know
>> some changes were coming but still. In the future when my llvm->tgsi work is
>> more
>> stable and I'll be pushing it to llvm upstream we really cannot make
>> breaking tgsi
>> changes like this.
>
> I thought I had sufficiently explained to you that the resource thing
> was going away -- it was never spec'd, and there were no users.

Yes I was aware that changes were coming.

> Perhaps I didn't make it clear enough though -- sorry. The reality is
> that TGSI is a living thing, and things change every so often. I don't
> think people would be comfortable with locking down TGSI in such a
> way.

Hmm, I do think that if we end up using a llvm-ir -> tgsi step in some
cases that we do need to lock TGSI down. Taken the RES thing for example,
it would have been pretty trivial actually to not nuke it, and instead just
add the image support next to it. Note I'm fine with making changes while
we are figuring this out, but once I start pushing the llvm bits to llvm
upstream (which is still months away) we are going to need some sort of
TGSI stability.

Regards,

Hans