[Mesa-dev] [PATCH v2 03/31] nvir: move common converter code in base class
Pierre Moreau
pierre.morrow at free.fr
Thu Jan 4 17:24:26 UTC 2018
With the comments below addressed, this patch is
Reviewed-by: Pierre Moreau <pierre.morrow at free.fr>
On 2018-01-04 — 16:01, Karol Herbst wrote:
> v2: remove TGSI related bits
>
> Signed-off-by: Karol Herbst <kherbst at redhat.com>
> ---
> src/gallium/drivers/nouveau/Makefile.sources | 2 +
> .../nouveau/codegen/nv50_ir_from_common.cpp | 107 +++++++++++++++++++++
> .../drivers/nouveau/codegen/nv50_ir_from_common.h | 58 +++++++++++
> .../drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp | 106 +-------------------
> src/gallium/drivers/nouveau/meson.build | 2 +
> 5 files changed, 172 insertions(+), 103 deletions(-)
> create mode 100644 src/gallium/drivers/nouveau/codegen/nv50_ir_from_common.cpp
> create mode 100644 src/gallium/drivers/nouveau/codegen/nv50_ir_from_common.h
>
> diff --git a/src/gallium/drivers/nouveau/Makefile.sources b/src/gallium/drivers/nouveau/Makefile.sources
> index 65f08c7d8d..fee5e59522 100644
> --- a/src/gallium/drivers/nouveau/Makefile.sources
> +++ b/src/gallium/drivers/nouveau/Makefile.sources
> @@ -115,6 +115,8 @@ NV50_CODEGEN_SOURCES := \
> codegen/nv50_ir_build_util.h \
> codegen/nv50_ir_driver.h \
> codegen/nv50_ir_emit_nv50.cpp \
> + codegen/nv50_ir_from_common.cpp \
> + codegen/nv50_ir_from_common.h \
> codegen/nv50_ir_from_tgsi.cpp \
> codegen/nv50_ir_graph.cpp \
> codegen/nv50_ir_graph.h \
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_common.cpp b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_common.cpp
> new file mode 100644
> index 0000000000..58e9ab311b
> --- /dev/null
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_common.cpp
> @@ -0,0 +1,107 @@
> +/*
> + * Copyright 2011 Christoph Bumiller
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include "codegen/nv50_ir_from_common.h"
> +
> +namespace nv50_ir {
> +
> +ConverterCommon::ConverterCommon(Program *prog, nv50_ir_prog_info *info)
> + : BuildUtil(prog),
> + info(info) {}
> +
> +ConverterCommon::Subroutine *
> +ConverterCommon::getSubroutine(unsigned ip)
> +{
> + std::map<unsigned, Subroutine>::iterator it = sub.map.find(ip);
> +
> + if (it == sub.map.end())
> + it = sub.map.insert(std::make_pair(
> + ip, Subroutine(new Function(prog, "SUB", ip)))).first;
> +
> + return &it->second;
> +}
> +
> +ConverterCommon::Subroutine *
> +ConverterCommon::getSubroutine(Function *f)
> +{
> + unsigned ip = f->getLabel();
> + std::map<unsigned, Subroutine>::iterator it = sub.map.find(ip);
> +
> + if (it == sub.map.end())
> + it = sub.map.insert(std::make_pair(ip, Subroutine(f))).first;
> +
> + return &it->second;
> +}
> +
> +uint8_t
> +ConverterCommon::translateInterpMode(const nv50_ir_varying *var, operation& op)
As you are only moving code around, I would not make any modifications to that
code, therefore I would keep the “struct” of “const struct nv50_ir_varying *”,
even if it does not matter in C++.
> +{
> + uint8_t mode = NV50_IR_INTERP_PERSPECTIVE;
> +
> + if (var->flat)
> + mode = NV50_IR_INTERP_FLAT;
> + else
> + if (var->linear)
> + mode = NV50_IR_INTERP_LINEAR;
> + else
> + if (var->sc)
> + mode = NV50_IR_INTERP_SC;
> +
> + op = (mode == NV50_IR_INTERP_PERSPECTIVE || mode == NV50_IR_INTERP_SC)
> + ? OP_PINTERP : OP_LINTERP;
> +
> + if (var->centroid)
> + mode |= NV50_IR_INTERP_CENTROID;
> +
> + return mode;
> +}
> +
> +void
> +ConverterCommon::handleUserClipPlanes()
> +{
> + Value *res[8];
> + int n, i, c;
> +
> + for (c = 0; c < 4; ++c) {
> + for (i = 0; i < info->io.genUserClip; ++i) {
> + Symbol *sym = mkSymbol(FILE_MEMORY_CONST, info->io.auxCBSlot,
> + TYPE_F32, info->io.ucpBase + i * 16 + c * 4);
> + Value *ucp = mkLoadv(TYPE_F32, sym, NULL);
> + if (c == 0)
> + res[i] = mkOp2v(OP_MUL, TYPE_F32, getScratch(), clipVtx[c], ucp);
> + else
> + mkOp3(OP_MAD, TYPE_F32, res[i], clipVtx[c], ucp, res[i]);
> + }
> + }
> +
> + const int first = info->numOutputs - (info->io.genUserClip + 3) / 4;
> +
> + for (i = 0; i < info->io.genUserClip; ++i) {
> + n = i / 4 + first;
> + c = i % 4;
> + Symbol *sym =
> + mkSymbol(FILE_SHADER_OUTPUT, 0, TYPE_F32, info->out[n].slot[c] * 4);
> + mkStore(OP_EXPORT, TYPE_F32, sym, NULL, res[i]);
> + }
> +}
> +
> +} // nv50_ir
// namespace nv50_ir
to keep the same style as the other C++ files in codegen
> diff --git a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_common.h b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_common.h
> new file mode 100644
> index 0000000000..16bc5e2576
> --- /dev/null
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_common.h
> @@ -0,0 +1,58 @@
> +/*
> + * Copyright 2011 Christoph Bumiller
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +#include "codegen/nv50_ir.h"
> +#include "codegen/nv50_ir_build_util.h"
> +
> +namespace nv50_ir {
> +
> +class ConverterCommon : public BuildUtil
> +{
> +public:
> + ConverterCommon(Program *, nv50_ir_prog_info *);
> +protected:
> + struct Subroutine
> + {
> + Subroutine(Function *f) : f(f) { }
> + Function *f;
> + ValueMap values;
> + };
> +
> + Subroutine *getSubroutine(unsigned ip);
> + Subroutine *getSubroutine(Function *);
> +
> + uint8_t translateInterpMode(const nv50_ir_varying *var, operation& op);
Same as above about the missing “struct” keyword.
> +
> + void handleUserClipPlanes();
> +
> + struct {
> + std::map<unsigned, Subroutine> map;
> + Subroutine *cur;
> + } sub;
> +
> + struct nv50_ir_prog_info *info;
> + Value *fragCoord[4];
> + Value *clipVtx[4];
> + Value *outBase; // base address of vertex out patch (for TCP)
Absolutely not critical, but I guess “zero” could be part of “ConverterCommon”.
(Not part of the required changes for the Rb)
> +};
> +
> +} // unnamed endspace
Comment should be updated: this is the end of namespace “nv50_ir”.
And I hadn’t even realised it says “endspace” instead of “namespace” :-D
> 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 34351dab51..ce14d2846e 100644
> --- a/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> +++ b/src/gallium/drivers/nouveau/codegen/nv50_ir_from_tgsi.cpp
> @@ -27,8 +27,8 @@
> #include <set>
>
> #include "codegen/nv50_ir.h"
> +#include "codegen/nv50_ir_from_common.h"
> #include "codegen/nv50_ir_util.h"
> -#include "codegen/nv50_ir_build_util.h"
>
> namespace tgsi {
>
> @@ -1634,7 +1634,7 @@ namespace {
>
> using namespace nv50_ir;
>
> -class Converter : public BuildUtil
> +class Converter : public ConverterCommon
> {
> public:
> Converter(Program *, const tgsi::Source *);
> @@ -1643,13 +1643,6 @@ public:
> bool run();
>
> private:
> - struct Subroutine
> - {
> - Subroutine(Function *f) : f(f) { }
> - Function *f;
> - ValueMap values;
> - };
> -
> Value *shiftAddress(Value *);
> Value *getVertexBase(int s);
> Value *getOutputBase(int s);
> @@ -1673,8 +1666,6 @@ private:
>
> bool handleInstruction(const struct tgsi_full_instruction *);
> void exportOutputs();
> - inline Subroutine *getSubroutine(unsigned ip);
> - inline Subroutine *getSubroutine(Function *);
> inline bool isEndOfSubroutine(uint ip);
>
> void loadProjTexCoords(Value *dst[4], Value *src[4], unsigned int mask);
> @@ -1686,7 +1677,6 @@ private:
> void handleTXQ(Value *dst0[4], enum TexQuery, int R);
> void handleFBFETCH(Value *dst0[4]);
> void handleLIT(Value *dst0[4]);
> - void handleUserClipPlanes();
>
> // Symbol *getResourceBase(int r);
> void getImageCoords(std::vector<Value *>&, int r, int s);
> @@ -1697,8 +1687,6 @@ private:
>
> void handleINTERP(Value *dst0[4]);
>
> - uint8_t translateInterpMode(const struct nv50_ir_varying *var,
> - operation& op);
> Value *interpolate(tgsi::Instruction::SrcRegister, int c, Value *ptr);
>
> void insertConvergenceOps(BasicBlock *conv, BasicBlock *fork);
> @@ -1730,12 +1718,6 @@ private:
>
> private:
> const tgsi::Source *code;
> - const struct nv50_ir_prog_info *info;
> -
> - struct {
> - std::map<unsigned, Subroutine> map;
> - Subroutine *cur;
> - } sub;
>
> uint ip; // instruction pointer
>
> @@ -1750,14 +1732,10 @@ private:
> DataArray oData; // TGSI_FILE_OUTPUT (if outputs in registers)
>
> Value *zero;
> - Value *fragCoord[4];
> - Value *clipVtx[4];
>
> Value *vtxBase[5]; // base address of vertex in primitive (for TP/GP)
> uint8_t vtxBaseValid;
>
> - Value *outBase; // base address of vertex out patch (for TCP)
> -
> Stack condBBs; // fork BB, then else clause BB
> Stack joinBBs; // fork BB, for inserting join ops on ENDIF
> Stack loopBBs; // loop headers
> @@ -1832,29 +1810,6 @@ Converter::makeSym(uint tgsiFile, int fileIdx, int idx, int c, uint32_t address)
> return sym;
> }
>
> -uint8_t
> -Converter::translateInterpMode(const struct nv50_ir_varying *var, operation& op)
> -{
> - uint8_t mode = NV50_IR_INTERP_PERSPECTIVE;
> -
> - if (var->flat)
> - mode = NV50_IR_INTERP_FLAT;
> - else
> - if (var->linear)
> - mode = NV50_IR_INTERP_LINEAR;
> - else
> - if (var->sc)
> - mode = NV50_IR_INTERP_SC;
> -
> - op = (mode == NV50_IR_INTERP_PERSPECTIVE || mode == NV50_IR_INTERP_SC)
> - ? OP_PINTERP : OP_LINTERP;
> -
> - if (var->centroid)
> - mode |= NV50_IR_INTERP_CENTROID;
> -
> - return mode;
> -}
> -
> Value *
> Converter::interpolate(tgsi::Instruction::SrcRegister src, int c, Value *ptr)
> {
> @@ -3085,30 +3040,6 @@ Converter::handleINTERP(Value *dst[4])
> }
> }
>
> -Converter::Subroutine *
> -Converter::getSubroutine(unsigned ip)
> -{
> - std::map<unsigned, Subroutine>::iterator it = sub.map.find(ip);
> -
> - if (it == sub.map.end())
> - it = sub.map.insert(std::make_pair(
> - ip, Subroutine(new Function(prog, "SUB", ip)))).first;
> -
> - return &it->second;
> -}
> -
> -Converter::Subroutine *
> -Converter::getSubroutine(Function *f)
> -{
> - unsigned ip = f->getLabel();
> - std::map<unsigned, Subroutine>::iterator it = sub.map.find(ip);
> -
> - if (it == sub.map.end())
> - it = sub.map.insert(std::make_pair(ip, Subroutine(f))).first;
> -
> - return &it->second;
> -}
> -
> bool
> Converter::isEndOfSubroutine(uint ip)
> {
> @@ -4149,35 +4080,6 @@ Converter::handleInstruction(const struct tgsi_full_instruction *insn)
> return true;
> }
>
> -void
> -Converter::handleUserClipPlanes()
> -{
> - Value *res[8];
> - int n, i, c;
> -
> - for (c = 0; c < 4; ++c) {
> - for (i = 0; i < info->io.genUserClip; ++i) {
> - Symbol *sym = mkSymbol(FILE_MEMORY_CONST, info->io.auxCBSlot,
> - TYPE_F32, info->io.ucpBase + i * 16 + c * 4);
> - Value *ucp = mkLoadv(TYPE_F32, sym, NULL);
> - if (c == 0)
> - res[i] = mkOp2v(OP_MUL, TYPE_F32, getScratch(), clipVtx[c], ucp);
> - else
> - mkOp3(OP_MAD, TYPE_F32, res[i], clipVtx[c], ucp, res[i]);
> - }
> - }
> -
> - const int first = info->numOutputs - (info->io.genUserClip + 3) / 4;
> -
> - for (i = 0; i < info->io.genUserClip; ++i) {
> - n = i / 4 + first;
> - c = i % 4;
> - Symbol *sym =
> - mkSymbol(FILE_SHADER_OUTPUT, 0, TYPE_F32, info->out[n].slot[c] * 4);
> - mkStore(OP_EXPORT, TYPE_F32, sym, NULL, res[i]);
> - }
> -}
> -
> void
> Converter::exportOutputs()
> {
> @@ -4219,13 +4121,11 @@ Converter::exportOutputs()
> }
> }
>
> -Converter::Converter(Program *ir, const tgsi::Source *code) : BuildUtil(ir),
> +Converter::Converter(Program *ir, const tgsi::Source *code) : ConverterCommon(ir, code->info),
> code(code),
> tgsi(NULL),
> tData(this), lData(this), aData(this), oData(this)
> {
> - info = code->info;
> -
> const unsigned tSize = code->fileSize(TGSI_FILE_TEMPORARY);
> const unsigned aSize = code->fileSize(TGSI_FILE_ADDRESS);
> const unsigned oSize = code->fileSize(TGSI_FILE_OUTPUT);
> diff --git a/src/gallium/drivers/nouveau/meson.build b/src/gallium/drivers/nouveau/meson.build
> index 5d679e1c5a..87bbc3ca9b 100644
> --- a/src/gallium/drivers/nouveau/meson.build
> +++ b/src/gallium/drivers/nouveau/meson.build
> @@ -129,6 +129,8 @@ files_libnouveau = files(
> 'codegen/nv50_ir_build_util.h',
> 'codegen/nv50_ir_driver.h',
> 'codegen/nv50_ir_emit_nv50.cpp',
> + 'codegen/nv50_ir_from_common.cpp',
> + 'codegen/nv50_ir_from_common.h',
> 'codegen/nv50_ir_from_tgsi.cpp',
> 'codegen/nv50_ir_graph.cpp',
> 'codegen/nv50_ir_graph.h',
> --
> 2.14.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180104/24ad315f/attachment-0001.sig>
More information about the mesa-dev
mailing list