[Mesa-dev] [PATCH 1/9] gm107/ir: Emit third src 'bound' and optional predicate output of SHFL

[Samuel Pitoiset]

On 04/08/2017 02:49 PM, Boyan Ding wrote:
> 2017-04-08 20:42 GMT+08:00 Samuel Pitoiset <samuel.pitoiset at gmail.com>:
>>
>>
>> On 04/08/2017 11:51 AM, Boyan Ding wrote:
>>>
>>> ---
>>>    .../drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp | 23
>>> ++++++++++++++++++----
>>>    1 file changed, 19 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
>>> b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
>>> index 6903132efa..4a741bf45b 100644
>>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
>>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_gm107.cpp
>>> @@ -966,11 +966,26 @@ CodeEmitterGM107::emitSHFL()
>>>          break;
>>>       }
>>>    -   /*XXX: what is this arg? hardcode immediate for now */
>>> -   emitField(0x22, 13, 0x1c03);
>>> -   type |= 2;
>>> +   switch (insn->src(2).getFile()) {
>>> +   case FILE_GPR:
>>> +      emitGPR(0x27, insn->src(2));
>>> +      break;
>>> +   case FILE_IMMEDIATE:
>>> +      emitIMMD(0x22, 13, insn->src(2));
>>> +      type |= 2;
>>> +      break;
>>> +   default:
>>> +      assert(!"invalid src2 file");
>>> +      break;
>>> +   }
>>
>>
>> This looks wrong to me. I think you need to check that src(2) exists because
>> you might hit the assert for DFDX (and friends) which uses SHFL.
> 
> I wasn't aware that this was used in gm107's lowering. That explains
> the 0x1c03 magic number here.
> 
> Do you think I shall leave 0x1c03 as default value or add the constant
> in nv50_ir_lowering_gm107.cpp?

Yeah, you probably need to emit src(2) there and it would be nice to 
figure out the magic number. :)

> 
> Thanks for pointing it out.
> 
> Cheers,
> Boyan Ding
> 
>>
>>
>>> +
>>> +   if (!insn->defExists(1))
>>> +      emitPRED(0x30);
>>> +   else {
>>> +      assert(insn->def(1).getFile() == FILE_PREDICATE);
>>> +      emitPRED(0x30, insn->def(1));
>>> +   }
>>>    -   emitPRED (0x30);
>>>       emitField(0x1e, 2, insn->subOp);
>>>       emitField(0x1c, 2, type);
>>>       emitGPR  (0x08, insn->src(0));
>>>
>>