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.
> 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).
-ilia
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
