On Mon, Jun 23, 2014 at 11:24 AM, Tobias Klausmann <[email protected]> wrote:
Please add a brief description of what your change does and how it achieves this. [Let me know if you're not comfortable writing that, and I can compose it for you.] Among other things, note that it fixes some piglit tests. And instead of saying "make it work like the others", try to provide an explanation of how it works and then mention that other drivers appear to work that way. > Signed-off-by: Tobias Klausmann <[email protected]> > --- > .../drivers/nouveau/codegen/nv50_ir_driver.h | 1 + > .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 27 > ++++++++++++++++++++-- > 2 files changed, 26 insertions(+), 2 deletions(-) > > diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h > b/src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h > index c885c8c..a8cc97c 100644 > --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h > +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_driver.h > @@ -189,6 +189,7 @@ struct nv50_ir_prog_info > uint16_t sampleInfoBase; /* base address for sample positions */ > uint8_t msInfoCBSlot; /* cX[] used for multisample info */ > uint16_t msInfoBase; /* base address for multisample info */ > + int32_t viewportID; viewportId to match the capitalization of the other ones (the reason you see e.g. CB is because it's a Const Buffer). Could you also place it next to sampleMask/fragDepth? And add a comment like they have. > } io; > > /* driver callback to assign input/output locations */ > diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp > b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp > index a0f1fe1..78e33f8 100644 > --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp > +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp > @@ -982,6 +982,9 @@ bool Source::scanDeclaration(const struct > tgsi_full_declaration *decl) > case TGSI_SEMANTIC_SAMPLEMASK: > info->io.sampleMask = i; > break; > + case TGSI_SEMANTIC_VIEWPORT_INDEX: > + info->io.viewportID = i; > + break; > default: > break; > } > @@ -1258,6 +1261,8 @@ private: > Stack joinBBs; // fork BB, for inserting join ops on ENDIF > Stack loopBBs; // loop headers > Stack breakBBs; // end of / after loop > + > + Value *viewport; > }; > > Symbol * > @@ -1555,8 +1560,15 @@ Converter::storeDst(const > tgsi::Instruction::DstRegister dst, int c, > mkOp2(OP_WRSV, TYPE_U32, NULL, dstToSym(dst, c), val); > } else > if (f == TGSI_FILE_OUTPUT && prog->getType() != Program::TYPE_FRAGMENT) { > - if (ptr || (info->out[idx].mask & (1 << c))) > - mkStore(OP_EXPORT, TYPE_U32, dstToSym(dst, c), ptr, val); > + > + if (ptr || (info->out[idx].mask & (1 << c))) { > + /* Save the viewport index into a scratch register so that it can be > + exported at EMIT time */ > + if (info->out[dst.getIndex(0)].sn == TGSI_SEMANTIC_VIEWPORT_INDEX > && viewport != NULL) > + mkOp1(OP_MOV, TYPE_U32, viewport, val); > + else > + mkStore(OP_EXPORT, TYPE_U32, dstToSym(dst, c), ptr, val); > + } > } else > if (f == TGSI_FILE_TEMPORARY || > f == TGSI_FILE_PREDICATE || > @@ -2523,6 +2535,14 @@ Converter::handleInstruction(const struct > tgsi_full_instruction *insn) > mkCvt(OP_CVT, dstTy, dst0[c], srcTy, fetchSrc(0, c)); > break; > case TGSI_OPCODE_EMIT: > + /* export the saved viewport index */ > + if (viewport != NULL) { > + Symbol *VPSym = mkSymbol(FILE_SHADER_OUTPUT, info->io.viewportID, Pretty sure you should just pass '0' as the second arg to mkSymbol -- it takes the file index, which makes sense for e.g. constbufs of which there can be many. Also, how about Symbol *vp or Symbol *vpSym to conform to the variable naming convention used throughout this file. > + TYPE_U32, > + info->out[info->io.viewportID].slot[0] * > 4); > + mkStore(OP_EXPORT, TYPE_U32, VPSym, NULL, viewport); > + } > + /* fallthrough */ > case TGSI_OPCODE_ENDPRIM: > // get vertex stream if specified (must be immediate) > src0 = tgsi.srcCount() ? > @@ -2952,6 +2972,9 @@ Converter::run() > mkOp1(OP_RCP, TYPE_F32, fragCoord[3], fragCoord[3]); > } > > + if (info->io.viewportID > 0) What if it's the first output? You need to initialize viewportId to 0xff somewhere... like right before the code that calls scanDeclarations() and then here check that it's < 0xff. [There can't be 255 outputs IIRC... the max is like 64 or 16 or something.] > + viewport = getScratch(); else viewport = NULL; > + > for (ip = 0; ip < code->scan.num_instructions; ++ip) { > if (!handleInstruction(&code->insns[ip])) > return false; > -- > 1.8.4.5 > _______________________________________________ mesa-dev mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/mesa-dev
