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

[Ilia Mirkin]

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.

> +
> +   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?

> +}
> +
>  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