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