[Mesa-dev] [PATCH] nvc0/ir: fix predication when the surface format doesn't mismatch

[Samuel Pitoiset]

On 05/20/2016 05:20 PM, Ilia Mirkin wrote:
> On Fri, May 20, 2016 at 11:03 AM, Samuel Pitoiset
> <samuel.pitoiset at gmail.com> wrote:
>> Initially to make sure the format doesn't mismatch and won't produce
>> out-of-bounds access, we checked that both formats have exactly the same
>> number of bytes. But this is actually wrong because the format specified
>> in the shader might be smaller. While we are at it, add the predicate
>> only when the number of components is not 0 instead of checking the
>> blocksize.
>
> Can you explain? I believe the format in shader has to match the
> format of the bound image.
>
> Separately, perhaps only do this check for reads/atomics, and not
> writes? (Since for writes, we don't really care, since the hw does
> it.)

Yeah, I think we don't need that check for surface stores. I'm going to 
update the patch.

>
>>
>> This fixes serious rendering issues in the UE4 demos (tested with
>> realistic and reflections).
>>
>> Signed-off-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
>> ---
>>  src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp | 4 ++--
>>  1 file changed, 2 insertions(+), 2 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 a1823a0..90578f5 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_lowering_nvc0.cpp
>> @@ -1839,9 +1839,9 @@ NVC0LoweringPass::processSurfaceCoordsNVE4(TexInstruction *su)
>>        int blockwidth = format->bits[0] + format->bits[1] +
>>                         format->bits[2] + format->bits[3];
>>
>> -      if (blockwidth >= 8) {
>> +      if (format->components != 0) {
>>           // make sure that the format doesn't mismatch
>> -         bld.mkCmp(OP_SET_OR, CC_NE, TYPE_U32, pred1->getDef(0),
>> +         bld.mkCmp(OP_SET_OR, CC_GT, TYPE_U32, pred1->getDef(0),
>>                     TYPE_U32, bld.loadImm(NULL, blockwidth / 8),
>>                     loadSuInfo32(ind, base + NVE4_SU_INFO_BSIZE),
>>                     pred1->getDef(0));
>> --
>> 2.8.2
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev