[Mesa-dev] [PATCH] nv50/ir: fetch indirect sources BEFORE the op that uses them
Samuel Pitoiset
samuel.pitoiset at gmail.com
Mon Jun 26 19:58:08 UTC 2017
Reviewed-by: Samuel Pitoiset <samuel.pitoiset at gmail.com>
On 06/24/2017 06:50 PM, Ilia Mirkin wrote:
> All the BuildUtil helpers just insert the operation into the current BB.
> So we have to take care that any fetchSrc() operations happen before the
> operation whose setIndirect() it goes into.
>
> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
> Cc: mesa-stable at lists.freedesktop.org
> ---
>
> I don't know how any of this really worked before. I suppose that pre-SSA, we
> could get lucky a lot and have it work out. But order of instructions matters.
>
> .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 51 ++++++++++++++--------
> 1 file changed, 32 insertions(+), 19 deletions(-)
>
> 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 0da069b2084..eadfca979ad 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> @@ -2632,6 +2632,10 @@ Converter::handleLOAD(Value *dst0[4])
> const int r = tgsi.getSrc(0).getIndex(0);
> int c;
> std::vector<Value *> off, src, ldv, def;
> + Value *ind = NULL;
> +
> + if (tgsi.getSrc(0).isIndirect(0))
> + ind = fetchSrc(tgsi.getSrc(0).getIndirect(0), 0, 0);
>
> switch (tgsi.getSrc(0).getFile()) {
> case TGSI_FILE_BUFFER:
> @@ -2658,8 +2662,8 @@ Converter::handleLOAD(Value *dst0[4])
>
> Instruction *ld = mkLoad(TYPE_U32, dst0[c], sym, off);
> ld->cache = tgsi.getCacheMode();
> - if (tgsi.getSrc(0).isIndirect(0))
> - ld->setIndirect(0, 1, fetchSrc(tgsi.getSrc(0).getIndirect(0), 0, 0));
> + if (ind)
> + ld->setIndirect(0, 1, ind);
> }
> break;
> case TGSI_FILE_IMAGE: {
> @@ -2681,8 +2685,8 @@ Converter::handleLOAD(Value *dst0[4])
> ld->tex.mask = tgsi.getDst(0).getMask();
> ld->tex.format = getImageFormat(code, r);
> ld->cache = tgsi.getCacheMode();
> - if (tgsi.getSrc(0).isIndirect(0))
> - ld->setIndirectR(fetchSrc(tgsi.getSrc(0).getIndirect(0), 0, NULL));
> + if (ind)
> + ld->setIndirectR(ind);
>
> FOR_EACH_DST_ENABLED_CHANNEL(0, c, tgsi)
> if (dst0[c] != def[c])
> @@ -2770,6 +2774,10 @@ Converter::handleSTORE()
> const int r = tgsi.getDst(0).getIndex(0);
> int c;
> std::vector<Value *> off, src, dummy;
> + Value *ind = NULL;
> +
> + if (tgsi.getDst(0).isIndirect(0))
> + ind = fetchSrc(tgsi.getDst(0).getIndirect(0), 0, 0);
>
> switch (tgsi.getDst(0).getFile()) {
> case TGSI_FILE_BUFFER:
> @@ -2792,8 +2800,8 @@ Converter::handleSTORE()
>
> Instruction *st = mkStore(OP_STORE, TYPE_U32, sym, off, fetchSrc(1, c));
> st->cache = tgsi.getCacheMode();
> - if (tgsi.getDst(0).isIndirect(0))
> - st->setIndirect(0, 1, fetchSrc(tgsi.getDst(0).getIndirect(0), 0, 0));
> + if (ind)
> + st->setIndirect(0, 1, ind);
> }
> break;
> case TGSI_FILE_IMAGE: {
> @@ -2811,8 +2819,8 @@ Converter::handleSTORE()
> st->tex.mask = tgsi.getDst(0).getMask();
> st->tex.format = getImageFormat(code, r);
> st->cache = tgsi.getCacheMode();
> - if (tgsi.getDst(0).isIndirect(0))
> - st->setIndirectR(fetchSrc(tgsi.getDst(0).getIndirect(0), 0, NULL));
> + if (ind)
> + st->setIndirectR(ind);
> }
> break;
> default:
> @@ -2881,6 +2889,10 @@ Converter::handleATOM(Value *dst0[4], DataType ty, uint16_t subOp)
> std::vector<Value *> srcv;
> std::vector<Value *> defv;
> LValue *dst = getScratch();
> + Value *ind = NULL;
> +
> + if (tgsi.getSrc(0).isIndirect(0))
> + ind = fetchSrc(tgsi.getSrc(0).getIndirect(0), 0, 0);
>
> switch (tgsi.getSrc(0).getFile()) {
> case TGSI_FILE_BUFFER:
> @@ -2890,23 +2902,21 @@ Converter::handleATOM(Value *dst0[4], DataType ty, uint16_t subOp)
> continue;
>
> Instruction *insn;
> - Value *off = fetchSrc(1, c), *off2 = NULL;
> + Value *off = fetchSrc(1, c);
> Value *sym;
> if (tgsi.getSrc(1).getFile() == TGSI_FILE_IMMEDIATE)
> sym = makeSym(tgsi.getSrc(0).getFile(), r, -1, c,
> tgsi.getSrc(1).getValueU32(c, info));
> else
> sym = makeSym(tgsi.getSrc(0).getFile(), r, -1, c, 0);
> - if (tgsi.getSrc(0).isIndirect(0))
> - off2 = fetchSrc(tgsi.getSrc(0).getIndirect(0), 0, 0);
> if (subOp == NV50_IR_SUBOP_ATOM_CAS)
> insn = mkOp3(OP_ATOM, ty, dst, sym, fetchSrc(2, c), fetchSrc(3, c));
> else
> insn = mkOp2(OP_ATOM, ty, dst, sym, fetchSrc(2, c));
> if (tgsi.getSrc(1).getFile() != TGSI_FILE_IMMEDIATE)
> insn->setIndirect(0, 0, off);
> - if (off2)
> - insn->setIndirect(0, 1, off2);
> + if (ind)
> + insn->setIndirect(0, 1, ind);
> insn->subOp = subOp;
> }
> for (int c = 0; c < 4; ++c)
> @@ -2929,8 +2939,8 @@ Converter::handleATOM(Value *dst0[4], DataType ty, uint16_t subOp)
> tex->tex.mask = 1;
> tex->tex.format = getImageFormat(code, r);
> tex->setType(ty);
> - if (tgsi.getSrc(0).isIndirect(0))
> - tex->setIndirectR(fetchSrc(tgsi.getSrc(0).getIndirect(0), 0, NULL));
> + if (ind)
> + tex->setIndirectR(ind);
>
> for (int c = 0; c < 4; ++c)
> if (dst0[c])
> @@ -3803,12 +3813,14 @@ Converter::handleInstruction(const struct tgsi_full_instruction *insn)
> break;
> case TGSI_OPCODE_RESQ:
> if (tgsi.getSrc(0).getFile() == TGSI_FILE_BUFFER) {
> + Value *ind = NULL;
> + if (tgsi.getSrc(0).isIndirect(0))
> + ind = fetchSrc(tgsi.getSrc(0).getIndirect(0), 0, 0);
> geni = mkOp1(OP_BUFQ, TYPE_U32, dst0[0],
> makeSym(tgsi.getSrc(0).getFile(),
> tgsi.getSrc(0).getIndex(0), -1, 0, 0));
> - if (tgsi.getSrc(0).isIndirect(0))
> - geni->setIndirect(0, 1,
> - fetchSrc(tgsi.getSrc(0).getIndirect(0), 0, 0));
> + if (ind)
> + geni->setIndirect(0, 1, ind);
> } else {
> assert(tgsi.getSrc(0).getFile() == TGSI_FILE_IMAGE);
>
> @@ -3821,10 +3833,11 @@ Converter::handleInstruction(const struct tgsi_full_instruction *insn)
> }
> texi->tex.r = tgsi.getSrc(0).getIndex(0);
> texi->tex.target = getImageTarget(code, texi->tex.r);
> - bb->insertTail(texi);
>
> if (tgsi.getSrc(0).isIndirect(0))
> texi->setIndirectR(fetchSrc(tgsi.getSrc(0).getIndirect(0), 0, NULL));
> +
> + bb->insertTail(texi);
> }
> break;
> case TGSI_OPCODE_IBFE:
>
More information about the mesa-dev
mailing list