Ilia Mirkin <imir...@alum.mit.edu> writes:
> On Mon, Feb 22, 2016 at 5:38 PM, Francisco Jerez <curroje...@riseup.net>
> wrote:
>> Ilia Mirkin <imir...@alum.mit.edu> writes:
>>
>>> On Mon, Feb 22, 2016 at 5:21 PM, Francisco Jerez <curroje...@riseup.net>
>>> wrote:
>>>> Ilia Mirkin <imir...@alum.mit.edu> writes:
>>>>
>>>>> On Mon, Feb 22, 2016 at 3:53 PM, Ilia Mirkin <imir...@alum.mit.edu> wrote:
>>>>>> On Mon, Feb 22, 2016 at 3:52 PM, Ilia Mirkin <imir...@alum.mit.edu>
>>>>>> wrote:
>>>>>>> This fixes atomic exchange with a r32f image, as needed by
>>>>>>> GL_OES_shader_atomic_exchange.
>>>>>>
>>>>>> Sorry, that should be GL_OES_shader_image_atomic of course.
>>>>>>
>>>>>>>
>>>>>>> Signed-off-by: Ilia Mirkin <imir...@alum.mit.edu>
>>>>>>> ---
>>>>>>> src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp | 6 ++++--
>>>>>>> 1 file changed, 4 insertions(+), 2 deletions(-)
>>>>>>>
>>>>>>> diff --git a/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp
>>>>>>> b/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp
>>>>>>> index 081dbad..e775cc0 100644
>>>>>>> --- a/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp
>>>>>>> +++ b/src/mesa/drivers/dri/i965/brw_fs_surface_builder.cpp
>>>>>>> @@ -156,8 +156,10 @@ namespace brw {
>>>>>>> const fs_reg tmp = bld.vgrf(BRW_REGISTER_TYPE_UD, n);
>>>>>>> bld.LOAD_PAYLOAD(tmp, srcs, n, 0);
>>>>>>>
>>>>>>> - return emit_send(bld, SHADER_OPCODE_TYPED_ATOMIC_LOGICAL,
>>>>>>> - addr, tmp, surface, dims, op, rsize);
>>>>>>> + return retype(
>>>>>>> + emit_send(bld, SHADER_OPCODE_TYPED_ATOMIC_LOGICAL,
>>>>>>> + addr, tmp, surface, dims, op, rsize),
>>>>>>> + src0.type);
>>>>>
>>>>> Actually I've been thinking a bit more about this... relying on
>>>>> src0.type won't fly. The surface's mesa_format needs to be passed
>>>>> through to emit_image_atomic, just like it is done for emit_image_load
>>>>> and emit_image_store. And I need to fix up the vec4 emitter. Curro,
>>>>> does that sound reasonable? Or should the retype be done at the
>>>>> emit_image_atomic callsites?
>>>>>
>>>> I doubt it's necessary to pass the surface format through.
>>>
>>> Well, you have to get the format from somewhere. You could easily have e.g.
>>>
>>> int foo, bar;
>>>
>>> foo = floatBitsAsInt(imageAtomicExchange(img, intBitsAsFloat(bar)));
>>>
>>> I believe in this scenario, src0.type is likely to be "int" instead of
>>> "float". Both of our fixes suffer from this problem.
>>
>> No, the src type is only dependent on the image type. If the image is
>> float you'll get a float.
>
> Ah yes, quite right:
>
> const fs_reg src0 = (info->num_srcs >= 3 ?
> retype(get_nir_src(instr->src[2]), base_type) :
> fs_reg());
>
> [where base_type is the image's type]
>
>>
>>>
>>>> Any reason
>>>> we shouldn't go with this series I sent half a year ago instead? (and you
>>>> actually commented to)
>>>>
>>>> https://lists.freedesktop.org/archives/mesa-dev/2015-August/091948.html
>>>> https://lists.freedesktop.org/archives/mesa-dev/2015-August/091949.html
>>>> https://lists.freedesktop.org/archives/mesa-dev/2015-August/091950.html
>>>> https://lists.freedesktop.org/archives/mesa-dev/2015-August/091951.html
>>>> https://lists.freedesktop.org/archives/mesa-dev/2015-August/091952.html
>>>
>>> Mainly that I forgot all about it :) Not sure if I commented on it
>>> back then, but I don't think we need a separate enable bit. Definitely
>>> not for TGSI (which is a lot more typeless), and it seems like not for
>>> i965 either. Also note that this behaviour is core in GLES 3.2 (and GL
>>> 4.5).
>>>
>> IIRC the series I sent already enabled the built-ins for GLES3.2 and
>> GL4.5 even though GLES3.2 had just been released. I don't think any
>> additional work is required.
>
> Probably. Either way it needs rebasing/etc... perhaps worth a resend?
>
The rebase wasn't too bad, but I can resend if you wish. I noticed I
had another branch I made just for you with the OES atomic enable bits
taken out :P, I didn't send it for review because other people seemed to
feel differently about that, I don't really personally care at this
point whether we have enable bits for it or not.> -ilia
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
