[Mesa-dev] [PATCH v2 2/9] nvc0/ir: Properly handle a "split form" of predicate destination

[Boyan Ding]

2017-04-10 9:31 GMT+08:00 Ilia Mirkin <imirkin at alum.mit.edu>:
> Wow, great find!
>
> On Sun, Apr 9, 2017 at 8:58 PM, Boyan Ding <boyan.j.ding at gmail.com> wrote:
>> GF100's ISA encoding has a weird form of predicate destination where its
>> 3 bits are split across whole the instruction. Use a dedicated setPDSTL
>> function instead of original defId which is incorrect in this case.
>> ---
>>  src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp | 13 +++++++++++--
>>  1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
>> index 5467447e35..d5a310f88c 100644
>> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
>> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_emit_nvc0.cpp
>> @@ -58,6 +58,7 @@ private:
>>     void setImmediateS8(const ValueRef&);
>>     void setSUConst16(const Instruction *, const int s);
>>     void setSUPred(const Instruction *, const int s);
>> +   inline void setPDSTL(const ValueDef&);
>>
>>     void emitCondCode(CondCode cc, int pos);
>>     void emitInterpMode(const Instruction *);
>> @@ -375,6 +376,14 @@ void CodeEmitterNVC0::setImmediateS8(const ValueRef &ref)
>>     code[0] |= (s8 >> 6) << 8;
>>  }
>>
>> +void CodeEmitterNVC0::setPDSTL(const ValueDef &def)
>> +{
>> +   uint32_t pred = (def.get() && def.getFile() != FILE_FLAGS ? DDATA(def).id : 7);
>
> Why not just == FILE_PREDICATE? Also, I don't think the outer parens do much.

Okay, will fix it.

>
>> +
>> +   code[0] |= (pred & 3) << 8;
>> +   code[1] |= !!(pred & 7) << 26;
>
> This always makes me nervous... how about
>
> (pred & 4) << (26 - 2)
>
> BTW, this should be pred & 4 in either case, no?

Yeah, should be pred & 4.

>
>> +}
>> +
>>  void
>>  CodeEmitterNVC0::emitForm_A(const Instruction *i, uint64_t opc)
>>  {
>> @@ -1873,7 +1882,7 @@ CodeEmitterNVC0::emitSTORE(const Instruction *i)
>>        if (i->src(0).getFile() == FILE_MEMORY_SHARED &&
>>            i->subOp == NV50_IR_SUBOP_STORE_UNLOCKED) {
>>           assert(i->defExists(0));
>> -         defId(i->def(0), 8);
>> +         setPDSTL(i->def(0));
>>        }
>>     }
>>
>> @@ -1945,7 +1954,7 @@ CodeEmitterNVC0::emitLOAD(const Instruction *i)
>>
>>     if (p >= 0) {
>>        if (targ->getChipset() >= NVISA_GK104_CHIPSET)
>> -         defId(i->def(p), 8);
>> +         setPDSTL(i->def(p));
>>        else
>>           defId(i->def(p), 32 + 18);
>>     }
>> --
>> 2.12.1
>>
>> _______________________________________________
>> mesa-dev mailing list
>> mesa-dev at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev