[Mesa-dev] [PATCH v2] nvc0/ir: fix indirect access for images

[Samuel Pitoiset]

On 05/22/2016 10:56 PM, Ilia Mirkin wrote:
> On Sun, May 22, 2016 at 4:55 PM, Samuel Pitoiset
> <samuel.pitoiset at gmail.com> wrote:
>> When the array doesn't start at 0 we need to account for su->tex.r.
>> While we are at it, make sure to avoid out of bounds access by masking
>> the index.
>>
>> This fixes GL45-CTS.shading_language_420pack.binding_image_array.
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> Reported-by: Dave Airlie <airlied at redhat.com>
>> ---
>>  .../nouveau/codegen/nv50_ir_lowering_nvc0.cpp      | 24 ++++++++++++++--------
>>  1 file changed, 16 insertions(+), 8 deletions(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
>> index 9b4bf82..b8c73e3 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
>> @@ -1677,10 +1677,14 @@ NVC0LoweringPass::processSurfaceCoordsNVE4(TexInstruction *su)
>>     adjustCoordinatesMS(su);
>>
>>     if (su->tex.rIndirectSrc >= 0) {
>> -      // FIXME: out of bounds
>> -      assert(su->tex.r == 0);
>> -      ind = bld.mkOp2v(OP_SHL, TYPE_U32, bld.getSSA(),
>> -                       su->getIndirectR(), bld.mkImm(6));
>> +      ind = su->getIndirectR();
>> +      if (su->tex.r > 0) {
>> +         ind = bld.mkOp2v(OP_AND, TYPE_U32, bld.getSSA(),
>> +                          bld.mkOp2v(OP_ADD, TYPE_U32, bld.getSSA(), ind,
>> +                                     bld.loadImm(NULL, su->tex.r)),
>> +                          bld.mkImm(7));
>> +      }
>> +      ind = bld.mkOp2v(OP_SHL, TYPE_U32, bld.getSSA(), ind, bld.mkImm(6));
>>     }
>>
>>     // calculate clamped coordinates
>> @@ -2026,10 +2030,14 @@ NVC0LoweringPass::processSurfaceCoordsNVC0(TexInstruction *su)
>>     Value *ind = NULL;
>>
>>     if (su->tex.rIndirectSrc >= 0) {
>> -      // FIXME: out of bounds
>> -      assert(su->tex.r == 0);
>> -      ind = bld.mkOp2v(OP_SHL, TYPE_U32, bld.getSSA(),
>> -                       su->getIndirectR(), bld.mkImm(6));
>> +      ind = su->getIndirectR();
>> +      if (su->tex.r > 0) {
>> +         ind = bld.mkOp2v(OP_AND, TYPE_U32, bld.getSSA(),
>
> But... the clamping needs to be done either way, no? I think this has
> to be outside of the if...

... right :)
>
>> +                          bld.mkOp2v(OP_ADD, TYPE_U32, bld.getSSA(), ind,
>> +                                     bld.loadImm(NULL, su->tex.r)),
>> +                          bld.mkImm(7));
>> +      }
>> +      ind = bld.mkOp2v(OP_SHL, TYPE_U32, bld.getSSA(), ind, bld.mkImm(6));
>>     }
>>
>>     // get surface coordinates
>> --
>> 2.8.2
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev