Re: [Mesa-dev] [PATCH] autotools: Deprecate the use of autotools
Am Freitag, den 11.01.2019, 17:28 -0500 schrieb Ilia Mirkin: > On Fri, Jan 11, 2019 at 5:12 PM Matt Turner > wrote: > > > [sorry Gert, not sure how to classify you] > > I think the vmware team (which largely maintains llvmpipe and svga) > is probably worth hearing from -- I believe they've largely stayed > out of it. But an ack/nack would be good. Also virgl isn't > represented, I believe. Dave gave some comments some time ago about issues he had with meson ( put him in CC). I'm also contributing to virgl, but this patch I didn't sign as Collaboran because this is more about my personal opinion that the removal of autotools without prior deprecation doesn't seem to me to be a good idea. I'm happy, btw, that all those in favour of removing autotools sooner than later went this step in my direction and have acknowledged this patch (and thanks to all the others too), but I will not push it with the strong NAK you gave, Ilia. To me consensus means that all who contribute significantly to the project (like you certainly do) agree or abstain, but don't object. Best, Gert > > > --- > > I think there's support for overriding the sole objection to this > > patch. > > > > To confirm: > > > > (1) The plan is to remove Autotools, perhaps after the 19.0 > > release > > > > (2) This patch's purpose is to ensure that everyone knows that > > Autotools will be going away (think: people who build Mesa > > as > > part of an automated process and wouldn't notice a > > deprecation > > warning unless it requires some action from them) > > If it's being removed _after_ the 19.0 release, does it make sense to > have a patch like this _in_ the 19.0 release? (Perhaps the answer is > `yes', but I'd still like to ask the question.) > > > > > (3) We expect all reasonable concerns about Meson to be > > resolved > > before Autotools is removed (e.g., reconfiguration > > problems, > > retrieving configuration command line, configuration status > > output, etc.) > > > > configure.ac | 13 + > > 1 file changed, 13 insertions(+) > > > > diff --git a/configure.ac b/configure.ac > > index e4d20054d5f..c7473d77eff 100644 > > --- a/configure.ac > > +++ b/configure.ac > > @@ -52,6 +52,19 @@ mingw*) > > ;; > > esac > > > > +AC_ARG_ENABLE(autotools, > > + [AS_HELP_STRING([--enable-autotools], > > + [Enable the use of this autotools based build > > configuration])], > > + [enable_autotools=$enableval], [enable_autotools=no]) > > + > > +if test "x$enable_autotools" != "xyes" ; then > > +AC_MSG_ERROR([the autotools build system has been deprecated > > in favour of > > +meson and will be removed eventually. For instructions on how > > to use meson > > +see https://www.mesa3d.org/meson.html. > > +If you still want to use the autotools build, then add -- > > enable-autotools > > +to the configure command line.]) > > +fi > > + > > # Support silent build rules, requires at least automake-1.11. > > Disable > > # by either passing --disable-silent-rules to configure or passing > > V=1 > > # to make > > -- > > 2.19.2 > > > > ___ > > mesa-dev mailing list > > mesa-dev@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] NIR + softfp64 problem
Hello, I'm trying to get soft-fp64 working with my experimental r600-nir backend, and thanks to Dave's contribution it is already tied in, some instructions are not yet supported by the backend, but when running the piglits I have already 24 tests passing. However, there are also 1099 test crashing, and skimming over the results this is mostly because of validation erros in nir_lower_doubles called with nir_lower_fp64_full_software. For instance, in fs-fract-dvec4 I get something like decl_var INTERP_MODE_NONE dvec4 p ... vec1 32 ssa_0 = deref_var &p (function dvec4) ... call __fadd64 ssa_77, ssa_78, ssa_79 vec1 64 ssa_80 = intrinsic load_deref (ssa_77) (0) /* access=0 */intrinsic store_deref (ssa_0, ssa_80) (15, 0) /* wrmask=xyzw */ /* access=0 */ error: src->ssa->num_components == num_components (../../../mesa/src/compiler/nir/nir_validate.c:206) ... with (src->ssa->num_components == 1) and (num_components == 4). i.e. a function returns a single value, but the target where to store it is a vector with four elements. I have no idea whether this is something triggered by the compile options I defined for nir, or whether this is independed from this, so any idea how to quell this would be very welcome. Many thanks, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] NIR + softfp64 problem
Hello Matt, Am Mittwoch, den 16.01.2019, 10:17 -0800 schrieb Matt Turner: > any idea how to quell this would be very welcome. > > It's required to scalarize fp64 operations before this lowering code > will work. It looks to me like it's trying to call __fadd64 with a > dvec4 argument, when the arguments are actually scalar. > > I think r600 is mostly vector-based? The soft-fp64 code probably > isn't ideally suited for that. I'd attempt to call > nir_lower_alu_to_scalar() before calling nir_lower_doubles(). That > should at minimum tell you whether my hypothesis is correct. Indeed, r600 I don't do nir_lower_alu_to_scalar() because it is more convenient for me, but with that the number of crashes goes down significantly, the remaining crashes seem to be in my own code. > > From there, maybe we could pass an options bitfield to > nir_lower_alu_to_scalar() to allow R600 to only lower fp64 operations > and not scalarize everything. I think this is the option I'll purse, I've touched that routine before before I disabled it completely. > Or, we could try to figure out how to > add vectorized versions of the soft-fp64 routines that R600 could use > directly. Unless other drivers can take advantage of this I don't think that this woule make much sense. To get the r600 nir backend performant I'd have to write some scheduler for the backend anyway (and this is still a long way to go). thanks for the pointers, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Add extension EXT_sRGB_write_control
Dear all, I'd like to push this series [1] to enable the extension EXT_sRGB_write control by the end of this week if there are no objections. I ran it trough the intel-ci [2] with only some unrelated unstable tests. many thanks for any reviews, Gert [1] https://gitlab.freedesktop.org/mesa/mesa/merge_requests/14 [2] https://mesa-ci.01.org/gerddie/builds/6/group/63a9f0ea7bb98050796b6 49e85481845 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Thoughts on fp64 for GLES?
Am Donnerstag, den 24.01.2019, 22:25 -0800 schrieb Stéphane Marchesin: > > Yes, it's for running virgl on top of GLES. To emulate fp64 in GL on > the guest side, we need fp64 on the host... BTW: we could also get it emulated from the guest side. When Elie (in CC) initially proposed the fp64 emulation series it was for r600 and TGSI was emitted. The created shaders are horribly long and it is certainly not performant, but if it's just for getting OpenGL 4.0 exposed it should be good enough. I'm not sure though how much work it would be to add this to the soft fp64 as it has now landed for NIR, though. Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] i965: Set flag for EXT_sRGB (review anyone from Intel?)
Hello Eric or Tapani (or anybody else working on the Intel driver), I wonder whether one of you could give me an R-B for the Intel patch [1] in this MR [2]. As far as I can see your CI [3] shows only unrelated changes. many thanks, Gert [1] https://gitlab.freedesktop.org/mesa/mesa/merge_requests/14/diffs?co mmit_id=99608c9a64912a24e6cffdb6bd6eb9e28efe336d [2] https://gitlab.freedesktop.org/mesa/mesa/merge_requests/14/commits [3] https://mesa-ci.01.org/gerddie/builds/6/group/63a9f0ea7bb98050796b6 49e85481845 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] relnotes: Add documentation for newly supported extensions
EXT_sRGB_write_control and EXT_texture_sRGB_R8 are now supported on all drivers that support sRGB. Signed-off-by: Gert Wollny CC: --- docs/relnotes/19.0.0.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/relnotes/19.0.0.html b/docs/relnotes/19.0.0.html index 1b4edd7ce76..33b35ab8cb0 100644 --- a/docs/relnotes/19.0.0.html +++ b/docs/relnotes/19.0.0.html @@ -41,8 +41,10 @@ TBD. GL_AMD_texture_texture4 on all GL 4.0 drivers. GL_EXT_shader_implicit_conversions on all drivers (ES extension). +GL_EXT_sRGB_write_control on all GL 3.0 drivers (ES extension). GL_EXT_texture_compression_bptc on all GL 4.0 drivers (ES extension). GL_EXT_texture_compression_rgtc on all GL 3.0 drivers (ES extension). +GL_EXT_texture_sRGB_R8 on all GL 3.0 drivers. GL_EXT_render_snorm on gallium drivers (ES extension). GL_EXT_texture_view on drivers supporting texture views (ES extension). GL_OES_texture_view on drivers supporting texture views (ES extension). -- 2.19.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Thoughts on fp64 for GLES?
> > > On Wed, Feb 13, 2019 at 11:09 AM Elie Tournier < > > > tournier.e...@gmail.com> wrote: > > > > [...] > > > > A solution would be to inline the > > > > function in GLSL but I'm scared than the following shader will > > > > be huge. From my adventures in R600 Evergreen NIR land I can say that the shaders with inlined functions get really huge, there were test cases in the piglits that resulted in >64k temporaries in ssa form (For using the NIR lowering fp64 to the emulation code one has to convert the shader to from vector to scalar operations first though, this may make the shader a bit longer too). > > > I suspect that's par for the course; the solution we pick will > > > not be pretty either way. > > > > > > Another option is to send TGSI f64 down the wire and lower in the > > > host, in virglrenderer, by emitting glsl helper functions which > > > implement fp64 and calling those. > > > > Options are then: > > > > a) do an Apple on evergreen - export GL4.x don't expose the > > ARB_gpu_shader_fp64 string, a lie, but a consistent lie. > > b) do fp32 for all fp64 - conversion to/from fp64 is still messy, > > but > > might cover at least some basic tests > > c) lower in guest - ugly tgsi, slow > > d) lower in host - emulate in virglrenderer, > > > > I'm actually considering (d) might not be the worst solution, we > > could in theory reuse the GLSL shaders code from mesa, and just > > link > > it in as another GLSL string. > > > > I would propose a fifth "option": > > - Do a) and/or b) for now (i.e expose a not-technically-conformant > 4.0) I wonder why this was never done for r600/evergreen, and unless conformance is the aim I'd say that this seems to be the best short term option. When I look at the state of D-GL on GLES hosts then I think there are more important issues that need to be fixed (e.g. KHR- GL33 failures) than sinking much time in implementing an extension that is used by nearly nobody. (a) is probably better in case someone wants to use this and expects correct results, because then no result is better then a result that is slightly off. > - Move to productize Zink, which: > 1) Targets Vulkan, where FP64 is an optional feature that doesn't > require an extension (VkPhysicalDeviceFeatures::shaderFloat64). > 2) Uses NIR, where lowering of FP64 is already supported for those > devices that doesn't support FP64. I second that given that Vulkan is the next step anyway, Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] r600: Regarding "Failed to build shader (translation from TGSI) #99349
Hello all, Hardware; Radeon 6850HD, Mesa: mesa 17.0.1 and git (sha 531887), llvm: 4.0.0 Playing a bit around with the Unreal Editor I was confronted with the same error message reported in #99349, i.e. "Failed to build shader (translation from TGSI). After some digging though the code I found that the TGSI code [1] of the offending shader reserves 151 temporaries so that the available 128 GPRs are already allocated right from the start, and when the operation "MUL TEMP[11], CONST[26], CONST[23]" is translated to byte code, both constants are read from the cfile region, because tgsi_split_constant could not move one constant to a proper GPR. As one can see in the TGSI dump [1], the shader does not really use 151 temporaries, only 40 are actually also addresses as source, to all the other temps values are just written once (assuming the the TGSI notation is OP DEST, SRC0, SRC1 ...). My questions are now: Does the GSLS-TGSI stage of the compilation do any optimizations? Specifically, should the unused temporaries be eliminated in that step and that I get this TGSI-dump is actually a bug in this compilation stage? (In the Gallium3D wikipedia article [2] it is written that there is a TGSI optimization stage.) As far as I understand there is a optimization pass done after the TGSI translation, but because of the nature of the problem the shader is rejected before. Would it make sense to implement a patch that would work around this problem by reserving some GORs to move constants to (and the temporary that is now ctx.temp_reg), and then test the number of allocated registers only after the byte code optimization? I partially implemented something like this [3] when I tried to find the source of the bug, so I could clean that up and propose a patch, so far the graphical output is clobbered though. many thank, Gert [1] https://bugs.freedesktop.org/attachment.cgi?id=131567 (12kb, xz compressed) [2] https://en.wikipedia.org/wiki/Gallium3D#Tungsten_Graphics_Shader_In frastructure [3] https://github.com/gerddie/mesa ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/1] r600g: work around shaders allocating too many superflous temporaries
Dear all, like I pointed out yesterday I've got a workaround for shaders that allocate too many temporaries after the TGSI pass that can actually be discarded in the sb pass but would fail before that in the tgsi-to-bc stage. The patch should not interfere with most shaders that worked before, the only problem could be with shaders that reserve a valid number of GPRs close to the limit of 124. Since I'm only starting to learn how the tgsi to bytecode translation works I'm not sure about the implications when temporaries are reserved in the kcache bank range (even though they are later discarded). Maybe r600_shader_from_tgsi() should actually return with an error the moment ctx.temp_reg >= 124 or r600_get_temp() > 124. I'm also aware the it would actually be better to optimize the shader in the TGSI representation in order to avoid the problem altogether, but before I can look into that I'll probably have t learn a lot more about the code. In any case, this patch fixes the problem I had with the Unreal Editor, the tests are still passing, piglit didn't show me any regressions, and so far I also didn't see breakage in other program using mesa. Many thanks for any comments, Gert Wollny (1): r600g: work around shaders allocating too many superflous temporaries src/gallium/drivers/r600/r600_shader.c | 52 +++--- 1 file changed, 42 insertions(+), 10 deletions(-) -- 2.13.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/1] r600g: work around shaders allocating too many superflous temporaries
Related bugs: https://bugs.freedesktop.org/show_bug.cgi?id=99349 https://bugs.freedesktop.org/show_bug.cgi?id=50338 1. Allocate ctx.temp_reg and a limited number of registers (R600_TEMP_REG_RESERVED=10) that are given out via r600_get_temp() before the temporaries of the TGSI are allocated. That makes it possible for tgsi_split_constants() allocate registers inside the proper GPR range, so that r600_asm.c:check_and_set_bank_swizzle doesn't fail. 2. Move the test for the register use limit (124) to after the optimization in r600_pipe_shader_create(). Add a test for a hard limit of 191 in tr600_shader_from_tgsi() though to avoid interference with reserved values. --- src/gallium/drivers/r600/r600_shader.c | 52 +++--- 1 file changed, 42 insertions(+), 10 deletions(-) diff --git a/src/gallium/drivers/r600/r600_shader.c b/src/gallium/drivers/r600/r600_shader.c index bdaf28ced2..d550f4cd7f 100644 --- a/src/gallium/drivers/r600/r600_shader.c +++ b/src/gallium/drivers/r600/r600_shader.c @@ -83,6 +83,13 @@ The compiler must issue the source argument to slots z, y, and x face_gpr.w = SampleID */ #define R600_SHADER_BUFFER_INFO_SEL (512 + R600_BUFFER_INFO_OFFSET / 16) + +/* Number of GPRs reserved before the temporaries in order to work around + problems with shaders that request too many temporaries that can be + optimized away in the sb pass. +*/ +#define R600_TEMP_REG_RESERVED 10 + static int r600_shader_from_tgsi(struct r600_context *rctx, struct r600_pipe_shader *pipeshader, union r600_shader_key key); @@ -216,6 +223,13 @@ int r600_pipe_shader_create(struct pipe_context *ctx, } } + if (shader->shader.bc.ngpr > 124) { + r = -ENOMEM; + R600_ERR("Shader GPR limit exceeded - shader requires %d registers.\n", +shader->shader.bc.ngpr); + goto error; + } + if (shader->gs_copy_shader) { if (dump) { // dump copy shader @@ -322,6 +336,7 @@ struct r600_shader_ctx { unsignedtype; unsignedfile_offset[TGSI_FILE_COUNT]; unsignedtemp_reg; + unsignedtemp_reg_highmem; const struct r600_shader_tgsi_instruction *inst_info; struct r600_bytecode*bc; struct r600_shader *shader; @@ -814,7 +829,11 @@ static inline int get_address_file_reg(struct r600_shader_ctx *ctx, int index) static int r600_get_temp(struct r600_shader_ctx *ctx) { - return ctx->temp_reg + ctx->max_driver_temp_used++; + if (ctx->max_driver_temp_used < R600_TEMP_REG_RESERVED) + return ctx->temp_reg + ctx->max_driver_temp_used++; + else + return ctx->temp_reg_highmem + ctx->max_driver_temp_used++ - + R600_TEMP_REG_RESERVED; } static int vs_add_primid_output(struct r600_shader_ctx *ctx, int prim_id_sid) @@ -2213,6 +2232,8 @@ static int generate_gs_copy_shader(struct r600_context *rctx, r600_bytecode_add_vtx(ctx.bc, &vtx); } ctx.temp_reg = i + 1; + ctx.temp_reg_highmem = ctx.temp_reg + R600_TEMP_REG_RESERVED; + for (ring = 3; ring >= 0; --ring) { bool enabled = false; for (i = 0; i < so->num_outputs; i++) { @@ -3065,8 +3086,11 @@ static int r600_shader_from_tgsi(struct r600_context *rctx, ctx.file_offset[TGSI_FILE_OUTPUT] = ctx.file_offset[TGSI_FILE_INPUT] + ctx.info.file_max[TGSI_FILE_INPUT] + 1; - ctx.file_offset[TGSI_FILE_TEMPORARY] = ctx.file_offset[TGSI_FILE_OUTPUT] + - ctx.info.file_max[TGSI_FILE_OUTPUT] + 1; + +ctx.temp_reg = ctx.file_offset[TGSI_FILE_OUTPUT] + + ctx.info.file_max[TGSI_FILE_OUTPUT] + 1; + +ctx.file_offset[TGSI_FILE_TEMPORARY] = ctx.temp_reg + R600_TEMP_REG_RESERVED; /* Outside the GPR range. This will be translated to one of the * kcache banks later. */ @@ -3081,19 +3105,19 @@ static int r600_shader_from_tgsi(struct r600_context *rctx, if (ctx.type == PIPE_SHADER_TESS_CTRL) { ctx.tess_input_info = ctx.bc->ar_reg + 3; ctx.tess_output_info = ctx.bc->ar_reg + 4; - ctx.temp_reg = ctx.bc->ar_reg + 5; + ctx.temp_reg_highmem = ctx.bc->ar_reg + 5; } else if (ctx.type == PIPE_SHADER_TESS_EVAL) { ctx.tess_input_info = 0; ctx.tess_output_info = ctx.bc->ar_reg + 3; - ctx.temp_reg = ctx.bc->ar_reg + 4; + ctx.temp_reg_highmem = ctx.bc->ar_reg + 4; } else if (ctx.type == PIPE_SHADER_GEOMETRY) { ctx.gs
Re: [Mesa-dev] [PATCH 0/1] r600g: work around shaders allocating too many superflous temporaries
Hello, I should probably add that I don't have write access to the mesa-git. Best, Gert Am Dienstag, den 30.05.2017, 14:14 +0200 schrieb Gert Wollny: > Dear all, > > like I pointed out yesterday I've got a workaround for shaders that > allocate > too many temporaries after the TGSI pass that can actually be > discarded in the > sb pass but would fail before that in the tgsi-to-bc stage. > > The patch should not interfere with most shaders that worked before, > the only > problem could be with shaders that reserve a valid number of GPRs > close to the > limit of 124. > > Since I'm only starting to learn how the tgsi to bytecode translation > works > I'm not sure about the implications when temporaries are reserved in > the > kcache bank range (even though they are later discarded). Maybe > r600_shader_from_tgsi() should actually return with an error > the moment ctx.temp_reg >= 124 orAm Dienstag, den 30.05.2017, 14:14 > +0200 schrieb Gert Wollny: > > Dear all, > > > > like I pointed out yesterday I've got a workaround for shaders that > > allocate > > too many temporaries after the TGSI pass that can actually be > > discarded in the > > sb pass but would fail before that in the tgsi-to-bc stage. > > > > The patch should not interfere with most shaders that worked > before, > > the only > > problem could be with shaders that reserve a valid number of GPRs > > close to the > > limit of 124. > > > > Since I'm only starting to learn how the tgsi to bytecode > translation > > works > > I'm not sure about the implications when temporaries are reserved > in > > the > > kcache bank range (even though they are later discarded). Maybe > > r600_shader_from_tgsi() should actually return with an error > > the moment ctx.temp_reg >= 124 or r600_get_temp() > 124. > > > > I'm also aware the it would actually be better to optimize the > shader > > in the > > TGSI representation in order to avoid the problem altogether, but > > before I > > can look into that I'll probably have t learn a lot more about the > > code. > > > > In any case, this patch fixes the problem I had with the Unreal > > Editor, the tests > > are still passing, piglit didn't show me any regressions, and so > far > > I also didn't > > see breakage in other program using mesa. > > > > Many thanks for any comments, > > Gert Wollny (1): > > r600g: work around shaders allocating too many superflous > > temporaries > > > > src/gallium/drivers/r600/r600_shader.c | 52 > > +++--- > > 1 file changed, 42 insertions(+), 10 deletions(-) r600_get_temp() > > 124. > > I'm also aware the it would actually be better to optimize the shader > in the TGSI representation in order to avoid the problem altogether, > but before I can look into that I'll probably have t learn a lot more > about the code. > > In any case, this patch fixes the problem I had with the Unreal > Editor, the tests are still passing, piglit didn't show me any > regressions, and so far I also didn't see breakage in other program > using mesa. > > Many thanks for any comments, > Gert Wollny (1): > r600g: work around shaders allocating too many superflous > temporaries > > src/gallium/drivers/r600/r600_shader.c | 52 > +++--- > 1 file changed, 42 insertions(+), 10 deletions(-) > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] regarding mesa/state_tracker/glsl_to_tgsi register renaming
Dear all, (and specifically to Dave Arlie), when looking at the TGSI code of shaders that fail on r600g because of too many temporaries used, I notes that often their lifetime is very short, like just assigned in one line and read the only time in the next line. Looking on the register-renaming code in mesa/st/st_glsl_to_tgsi.cpp I also notes that the register life time estimation is very conservative, i.e. registers that are assigned and used within a loop get a life time over the whole loop range. Because of this, and also because I'm new to mesa development and would like to get a bit more knowledge about the mesa code base, I thought about re-implementing the register renaming. My plan is to do a complete rewrite that can be run instead of the current implementation like by setting an environment variable, I'd also like to propose a proper patch for the UE4 editor problem I could only solve with a rather hackish solution [1] :) Now, I've seen that you, Dave, are already working on code related to st_glsl_to_tgsi, so in order not to interfere with your work, or duplicate it, I wanted to inquire whether you are already working or planning to work on the register renaming part? many thanks, Gert [1] https://patchwork.freedesktop.org/patch/158872/ ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/3] [RFC] mesa/st: glsl_to_tgsi: improved temp-reg lifetime estimation
Dear all, as I wrote before, I was looking into the temporary register renaming. This series of patches implements a new approach that achieves a tigher estimation of the life time of the temporaries, and as a result the Piano and Voloplosion benchmarks implemented in gputest [1] now work. Before they failed with "r600_pipe_shader_create - translation from TGSI failed!" Piglit shows 7 fixes and 6 regressions compared to git 8fac894f, but they don't seem to be related to shaders. I've also tested other programs like the unignie-* benchmarks and they didn't show regressions. I think that the patch will need a few more iterations to remove code duplication and generally adhere to the mesa style, but I think it is atthe point where I could need a bit of feedback to get it into shape to be acceptable, and I'd also like to mention that since I'm new to mesa this I have no commit rights. many thanks, Gert [1] http://www.geeks3d.com/gputest/ Gert Wollny (3): mesa/st: glsl_to_tgsi move some helper classes to extra files mesa/st: glsl_to_tgsi Implement a new lifetime tracker for temporaries mesa/st: glsl_to_tgsi: tie in the new register renaming approach configure.ac | 1 + src/mesa/Makefile.am | 4 +- src/mesa/Makefile.sources | 4 + src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 302 +--- src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp | 241 +++ src/mesa/state_tracker/st_glsl_to_tgsi_private.h | 135 .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 551 ++ .../state_tracker/st_glsl_to_tgsi_temprename.h | 114 +++ src/mesa/state_tracker/tests/Makefile.am | 40 ++ src/mesa/state_tracker/tests/st-renumerate-test| 210 ++ .../tests/test_glsl_to_tgsi_lifetime.cpp | 789 + 11 files changed, 2104 insertions(+), 287 deletions(-) create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_private.h create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_temprename.h create mode 100644 src/mesa/state_tracker/tests/Makefile.am create mode 100755 src/mesa/state_tracker/tests/st-renumerate-test create mode 100644 src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp -- 2.13.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/3] mesa/st: glsl_to_tgsi: tie in the new register renaming approach
This patch replaces the old register livetime estimation with the new approach. --- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 17 +++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index 0e7f4b646a..b76ad42536 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -55,10 +55,11 @@ #include "st_glsl_types.h" #include "st_nir.h" #include "st_shader_cache.h" -#include "st_glsl_to_tgsi_private.h" +#include "st_glsl_to_tgsi_temprename.h" #include "util/hash_table.h" #include +#include #define PROGRAM_ANY_CONST ((1 << PROGRAM_STATE_VAR) |\ (1 << PROGRAM_CONSTANT) | \ @@ -323,6 +324,7 @@ public: void merge_two_dsts(void); void merge_registers(void); + void merge_registers_alternative(void); void renumber_registers(void); void emit_block_mov(ir_assignment *ir, const struct glsl_type *type, @@ -5042,6 +5044,17 @@ glsl_to_tgsi_visitor::merge_two_dsts(void) } } +void +glsl_to_tgsi_visitor::merge_registers_alternative(void) +{ + rename_reg_pair proto ={false, 0}; + std::vector renames(this->next_temp, proto); + tgsi_temp_lifetime analysis(&this->instructions, this->next_temp); + auto lt = analysis.get_lifetimes(); + evaluate_remapping(lt, renames); + rename_temp_registers(&renames[0]); +} + /* Merges temporary registers together where possible to reduce the number of * registers needed to run a program. * @@ -6492,7 +6505,7 @@ get_mesa_program_tgsi(struct gl_context *ctx, v->merge_two_dsts(); if (!skip_merge_registers) - v->merge_registers(); + v->merge_registers_alternative(); v->renumber_registers(); /* Write the END instruction. */ -- 2.13.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/3] mesa/st: glsl_to_tgsi Implement a new lifetime tracker for temporaries
This patch adds new classes and tests to implement a tracker for the life time of temporary registers for the register renaming stage of glsl_to_tgsi. The tracker aims at estimating the shortest possible life time for each register. The code base requires c++11, the flag is propagated from the LLVM_CXXFLAGS. --- configure.ac | 1 + src/mesa/Makefile.am | 4 +- src/mesa/Makefile.sources | 2 + .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 551 ++ .../state_tracker/st_glsl_to_tgsi_temprename.h | 114 +++ src/mesa/state_tracker/tests/Makefile.am | 40 ++ src/mesa/state_tracker/tests/st-renumerate-test| 210 ++ .../tests/test_glsl_to_tgsi_lifetime.cpp | 789 + 8 files changed, 1709 insertions(+), 2 deletions(-) create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_temprename.h create mode 100644 src/mesa/state_tracker/tests/Makefile.am create mode 100755 src/mesa/state_tracker/tests/st-renumerate-test create mode 100644 src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp diff --git a/configure.ac b/configure.ac index f379ba8573..579e159420 100644 --- a/configure.ac +++ b/configure.ac @@ -2827,6 +2827,7 @@ AC_CONFIG_FILES([Makefile src/mesa/drivers/osmesa/osmesa.pc src/mesa/drivers/x11/Makefile src/mesa/main/tests/Makefile + src/mesa/state_tracker/tests/Makefile src/util/Makefile src/util/tests/hash_table/Makefile src/vulkan/Makefile]) diff --git a/src/mesa/Makefile.am b/src/mesa/Makefile.am index 53f311d2a9..72ffd61212 100644 --- a/src/mesa/Makefile.am +++ b/src/mesa/Makefile.am @@ -19,7 +19,7 @@ # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS # IN THE SOFTWARE. -SUBDIRS = . main/tests +SUBDIRS = . main/tests state_tracker/tests if HAVE_XLIB_GLX SUBDIRS += drivers/x11 @@ -101,7 +101,7 @@ AM_CFLAGS = \ $(VISIBILITY_CFLAGS) \ $(MSVC2013_COMPAT_CFLAGS) AM_CXXFLAGS = \ - $(LLVM_CFLAGS) \ +$(LLVM_CXXFLAGS) \ $(VISIBILITY_CXXFLAGS) \ $(MSVC2013_COMPAT_CXXFLAGS) diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources index 4450d80090..908d1acff6 100644 --- a/src/mesa/Makefile.sources +++ b/src/mesa/Makefile.sources @@ -507,6 +507,8 @@ STATETRACKER_FILES = \ state_tracker/st_glsl_to_tgsi.h \ state_tracker/st_glsl_to_tgsi_private.cpp \ state_tracker/st_glsl_to_tgsi_private.h \ +state_tracker/st_glsl_to_tgsi_temprename.cpp \ + state_tracker/st_glsl_to_tgsi_temprename.h \ state_tracker/st_glsl_types.cpp \ state_tracker/st_glsl_types.h \ state_tracker/st_manager.c \ diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp new file mode 100644 index 00..389a4b6b5f --- /dev/null +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp @@ -0,0 +1,551 @@ +/* + * Copyright © 2017 Gert Wollny + * + * 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 (including the next + * paragraph) 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 "st_glsl_to_tgsi_temprename.h" +#include +#include +#include +#include +#include + + +using std::vector; +using std::stack; +using std::shared_ptr; +using std::weak_ptr; +using std::pair; +using std::make_pair; +using std::make_shared; +using std::numeric_limits; + +tgsi_temp_lifetime::tgsi_temp_lifetime(exec_list *instructions, int ntemps): + lifetimes(ntemps) +{ + evaluate(instructions); +} + +const std::vector >& tgsi_temp_lifetime::get_lifetimes() const +{ + return lifetimes; +} + +void tgsi_temp_lifetime::evaluate(exec_list *instructions) +{ + int i = 0; +
[Mesa-dev] [PATCH 1/3] mesa/st: glsl_to_tgsi move some helper classes to extra files
ize) return size_swizzles[size - 1]; } -static bool -is_resource_instruction(unsigned opcode) -{ - switch (opcode) { - case TGSI_OPCODE_RESQ: - case TGSI_OPCODE_LOAD: - case TGSI_OPCODE_ATOMUADD: - case TGSI_OPCODE_ATOMXCHG: - case TGSI_OPCODE_ATOMCAS: - case TGSI_OPCODE_ATOMAND: - case TGSI_OPCODE_ATOMOR: - case TGSI_OPCODE_ATOMXOR: - case TGSI_OPCODE_ATOMUMIN: - case TGSI_OPCODE_ATOMUMAX: - case TGSI_OPCODE_ATOMIMIN: - case TGSI_OPCODE_ATOMIMAX: - return true; - default: - return false; - } -} - -static unsigned -num_inst_dst_regs(const glsl_to_tgsi_instruction *op) -{ - return op->info->num_dst; -} - -static unsigned -num_inst_src_regs(const glsl_to_tgsi_instruction *op) -{ - return op->info->is_tex || is_resource_instruction(op->op) ? - op->info->num_src - 1 : op->info->num_src; -} glsl_to_tgsi_instruction * glsl_to_tgsi_visitor::emit_asm(ir_instruction *ir, unsigned op, diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp new file mode 100644 index 00..337f21cf79 --- /dev/null +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp @@ -0,0 +1,241 @@ +/* + * Copyright © 2010 Intel Corporation + * Copyright © 2011 Bryan Cain + * Copyright © 2017 Gert Wollny + * + * 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 (including the next + * paragraph) 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 "st_glsl_to_tgsi_private.h" +#include +#include + +using std::vector; + +extern int swizzle_for_size(int size); + +static int swizzle_for_type(const glsl_type *type, int component = 0) +{ + unsigned num_elements = 4; + + if (type) { + type = type->without_array(); + if (type->is_scalar() || type->is_vector() || type->is_matrix()) + num_elements = type->vector_elements; + } + + int swizzle = swizzle_for_size(num_elements); + assert(num_elements + component <= 4); + + swizzle += component * MAKE_SWIZZLE4(1, 1, 1, 1); + return swizzle; +} + + + +st_src_reg::st_src_reg(gl_register_file file, int index, const glsl_type *type, + int component, unsigned array_id) +{ + assert(file != PROGRAM_ARRAY || array_id != 0); + this->file = file; + this->index = index; + this->swizzle = swizzle_for_type(type, component); + this->negate = 0; + this->abs = 0; + this->index2D = 0; + this->type = type ? type->base_type : GLSL_TYPE_ERROR; + this->reladdr = NULL; + this->reladdr2 = NULL; + this->has_index2 = false; + this->double_reg2 = false; + this->array_id = array_id; + this->is_double_vertex_input = false; +} + +st_src_reg::st_src_reg(gl_register_file file, int index, enum glsl_base_type type) +{ + assert(file != PROGRAM_ARRAY); /* need array_id > 0 */ + this->type = type; + this->file = file; + this->index = index; + this->index2D = 0; + this->swizzle = SWIZZLE_XYZW; + this->negate = 0; + this->abs = 0; + this->reladdr = NULL; + this->reladdr2 = NULL; + this->has_index2 = false; + this->double_reg2 = false; + this->array_id = 0; + this->is_double_vertex_input = false; +} + +st_src_reg::st_src_reg(gl_register_file file, int index, enum glsl_base_type type, int index2D) +{ + assert(file != PROGRAM_ARRAY); /* need array_id > 0 */ + this->type = type; + this->file = file; + this->index = index; + this->index2D = index2D; + this->swizzle = SWIZZLE_XYZW; + this->negate = 0; + this->abs = 0; + this->reladdr = NULL; + this->reladdr2 = NULL; + this->has_index2 = false; + this->double_reg2 = false; + this->array_id = 0; + this->is_double_vertex_input = false; +} + +st_src_reg::st_src_reg() +{ + this->type = GLSL_TYPE_ERROR; + this->file = PROGRAM_UNDEFINED; + this->index = 0; + thi
Re: [Mesa-dev] [PATCH 0/3] [RFC] mesa/st: glsl_to_tgsi: improved temp-reg lifetime estimation
Hello Marek, thanks for chiming in. Am Sonntag, den 11.06.2017, 16:15 +0200 schrieb Marek Olšák: > Also, I don't know if people will like that it uses STL. I personally > have no issue with that as long as it doesn't break apps (e.g. the > STL shipped with apps should be the same as the STL shipped with the > distribution). Well, on Linux I would take it for granted that the STL used to run the code is the same like the one the code was compiled with, and there are already quite some places in the mesa code where STL constructs are used (if that wounld't have been the case, then I would tried to avoid the STL). I am actually more concerned that propagating the C++11 requirement to the whole of src/mesa might not be welcomed (although everything compiles and runs fine). > On Sun, Jun 11, 2017 at 4:12 PM, Marek Olšák > wrote: > > Hi Gert, > > > > Have you measured the CPU overhead of the new code? So far no, I guess one would do that with the shader-db to get reasonable complex shaders, but I only have a r600 based card so I'm not sure whether I can run this. In any case, tomorrow I will take a look into this. Best, Gert > > > > Marek > > > > On Sat, Jun 10, 2017 at 1:15 AM, Gert Wollny > > wrote: > > > Dear all, > > > > > > as I wrote before, I was looking into the temporary register > > > renaming. > > > > > > This series of patches implements a new approach that achieves a > > > tigher > > > estimation of the life time of the temporaries, and as a result > > > the Piano > > > and Voloplosion benchmarks implemented in gputest [1] now work. > > > Before > > > they failed with "r600_pipe_shader_create - translation from TGSI > > > failed!" > > > > > > Piglit shows 7 fixes and 6 regressions compared to git 8fac894f, > > > but they don't > > > seem to be related to shaders. I've also tested other programs > > > like the unignie-* > > > benchmarks and they didn't show regressions. > > > > > > I think that the patch will need a few more iterations to remove > > > code duplication > > > and generally adhere to the mesa style, but I think it is atthe > > > point where I could > > > need a bit of feedback to get it into shape to be acceptable, and > > > I'd also like to > > > mention that since I'm new to mesa this I have no commit rights. > > > > > > many thanks, > > > Gert > > > > > > [1] http://www.geeks3d.com/gputest/ > > > > > > Gert Wollny (3): > > > mesa/st: glsl_to_tgsi move some helper classes to extra files > > > mesa/st: glsl_to_tgsi Implement a new lifetime tracker for > > > temporaries > > > mesa/st: glsl_to_tgsi: tie in the new register renaming > > > approach > > > > > > configure.ac | 1 + > > > src/mesa/Makefile.am | 4 +- > > > src/mesa/Makefile.sources | 4 + > > > src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 302 + > > > --- > > > src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp | 241 +++ > > > src/mesa/state_tracker/st_glsl_to_tgsi_private.h | 135 > > > .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 551 > > > ++ > > > .../state_tracker/st_glsl_to_tgsi_temprename.h | 114 +++ > > > src/mesa/state_tracker/tests/Makefile.am | 40 ++ > > > src/mesa/state_tracker/tests/st-renumerate-test| 210 ++ > > > .../tests/test_glsl_to_tgsi_lifetime.cpp | 789 > > > + > > > 11 files changed, 2104 insertions(+), 287 deletions(-) > > > create mode 100644 > > > src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp > > > create mode 100644 > > > src/mesa/state_tracker/st_glsl_to_tgsi_private.h > > > create mode 100644 > > > src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp > > > create mode 100644 > > > src/mesa/state_tracker/st_glsl_to_tgsi_temprename.h > > > create mode 100644 src/mesa/state_tracker/tests/Makefile.am > > > create mode 100755 src/mesa/state_tracker/tests/st-renumerate- > > > test > > > create mode 100644 > > > src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp > > > > > > -- > > > 2.13.0 > > > > > > ___ > > > mesa-dev mailing list > > > mesa-dev@lists.freedesktop.org > > > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/3] [RFC] mesa/st: glsl_to_tgsi: improved temp-reg lifetime estimation
Am Sonntag, den 11.06.2017, 19:21 +0200 schrieb Gert Wollny: > > > > > > > Have you measured the CPU overhead of the new code? > > So far no, I guess one would do that with the shader-db to get > reasonable complex shaders, but I only have a r600 based card so I'm > not sure whether I can run this. In any case, tomorrow I will take a > look into this. I did runs of the shader-db/run program with valgrind/callgrind Here the original merge_registers reports 0.21% and my code reports 0.50%. If it is important to cut down on these addes 0.3%, I think, I can eliminate 0.1% by changing the implementation to not used so many dynamically allocated objects, but beyond that it will be difficult. Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/3] [RFC] mesa/st: glsl_to_tgsi: improved temp-reg lifetime estimation
Am Montag, den 12.06.2017, 15:44 +0900 schrieb Michel Dänzer: > On 10/06/17 08:15 AM, Gert Wollny wrote: > > > > Piglit shows 7 fixes and 6 regressions compared to git 8fac894f, > > but they don't > > seem to be related to shaders. > > Which tests regressed (maybe you can put up a piglit HTML summary > somewhere generated from a run with and without your patches)? Do > they consistently pass without your patches and fail with them? I had to redo the results, because I realized that I had compared the system mesa version (with EGL support) versus the test version (without EGL support). Now both tested versions were configure with the same options and I run both versions two times. The result diff of the quick test are: piglit summary console -d results/o1 results/o2 results/n1 results/n2 glx/glx-multithread-texture: pass pass fail fail glx/glx-visuals-stencil: fail fail pass pass glx/glx_arb_sync_control/timing -fullscreen -divisor 1: pass fail pass fail glx/glx_arb_sync_control/timing -fullscreen -divisor 2: pass fail fail warn glx/glx_arb_sync_control/timing -fullscreen -msc-delta 1: fail fail warn fail glx/glx_arb_sync_control/timing -fullscreen -msc-delta 2: fail fail fail pass glx/glx_arb_sync_control/timing -msc-delta 2: warn fail pass fail glx/glx_arb_sync_control/timing -waitformsc -msc-delta 2: fail pass pass fail spec/arb_shader_bit_encoding/execution/and-clamp: fail fail pass fail spec/arb_shading_language_420pack/active sampler conflict: fail fail pass fail spec/glsl-1.50/execution/variable-indexing/gs-input-array-vec2-index- rd: fail fail pass pass spec/nv_conditional_render/drawpixels: fail pass fail pass spec/nv_conditional_render/vertex_array: fail pass pass pass summary: name: o1 o2 n1 n2 -- -- -- -- pass: 31583 31584 31588 31585 fail: 1454 1454 1449 1452 crash: 5 5 5 5 skip: 17356 17356 17356 17356 timeout: 0 0 0 0 warn: 14 13 14 14 incomplete: 0 0 0 0 dmesg-warn: 0 0 0 0 dmesg-fail: 0 0 0 0 changes: 0 6 9 9 fixes: 0 3 7 3 regressions: 0 3 2 6 total: 50412 50412 50412 50412 Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/3] [RFC] mesa/st: glsl_to_tgsi: improved temp-reg lifetime estimation
Am Montag, den 12.06.2017, 12:28 +0200 schrieb Nicolai Hähnle: > On 10.06.2017 01:15, Gert Wollny wrote: > > > Plenty of style issues aside, can you explain where and why you get > tighter lifetimes? In the original code if a temporary is used within a loop it gets the whole life time of the loop assigned. With this patch I track in more detail where a temporary is accesses and base the lifetime on this: For instance, if a variable is first unconditionally written and later read for the last time in the same scope (loop, if, or switch branch), then the lifetime can be restricted to that first-written - last-read range. The code gets more complex because it tries to resolve this also for nested scopes, and one also has to take care about whether a variable is written only conditionally within a loop, or conditionally read before it is written (in the source code sense, but not in the program flow sense). Shaders that profit from this better lifetime estimation are the ones that have many short living values within long loops, think for () { float4 t[1] = f(in2); float4 t[2] = g(in1); float4 t[3] = op(t[1], t[2]); ... sum += t[200]; } Here the old code would keep all of the t[i] alive for the whole loop, and in fact with the GpuTest Piano benchmark I have seen a shader with >2000 temporaries where many were used in a loop but only required for two or three lines so that my code could merge them to less then 100 temporaries, and this made it possible for the tgsi to bytecode layer in r600g to actually translate the shader. Best, Gert PS: Regarding style, I am fully aware that I have to iterate over this code a few more times. I tried to adhere to the way the existing code represents itself to me, but I'm happy to listen to any advise I can get. In any case, I though it might be best to send this patch out now for discussion. Now, with the unit tests in place I will rework it and focus also more on style questions. One thing that comes up immediately up is that I will try to reduce the use of dynamically allocated memory, since 60% of the run time of my code is with memory allocation and de-allocation. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/3] [RFC] mesa/st: glsl_to_tgsi: improved temp-reg lifetime estimation
Hello Nicolai, Am Montag, den 12.06.2017, 12:17 +0200 schrieb Nicolai Hähnle: > > spec/arb_shader_bit_encoding/execution/and-clamp: fail fail pass > > fail > > > > > > > It's disconcerting that you have tests here whose pass status is > unstable. Those tests really should be deterministic. When I run only these tests ("all" and specified with -t ) then they always pass (= three times in a row). spec/glsl-1.50/execution/variable-indexing/gs-input-array-vec2- > > index- > > rd: fail fail pass pass > > This one actually now passes because of the patch (before it failed because it needed 125 registers, and only 124 are free to be used. > > spec/arb_shading_language_420pack/active sampler conflict: fail > > fail pass fail > > > > spec/nv_conditional_render/drawpixels: fail pass fail pass > > > > spec/nv_conditional_render/vertex_array: fail pass pass pass > These, however, are unstable, independent on whether my patches are applied or not. Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/3] [RFC] mesa/st: glsl_to_tgsi: improved temp-reg lifetime estimation
Am Montag, den 12.06.2017, 21:00 +0200 schrieb Nicolai Hähnle: Thanks for you comments, although I do not agree with most of them. > > > Okay. I think you should seriously re-think your algorithm in a way > > that makes it a more natural evolution from the algorithm that's > > already there. Well, when I look at the current algorithm than I don't really see much to evolve from: The tracking of loops is minimal and it actually only uses the outer loop to assign life times. In any case, 80% of this code I already re-used (i.e. the loop over the instructions and iterating over the registers). > > Basically, the current algorithm tracks (first_write, last_read), > > so think about what you need to track in order to obtain a single- > > pass algorithm that computes lifetime (first, last) for every > > temporary. I IMHO a single pass algorithm is not better then what I do now. Especially with nested loops a single pass algorithm will be more complicated. Think e.g. ... BEGIN_LOOP ... BEGIN_LOOP a = ... b = ... c = a OP b; END_LOOP ... /* more processing that doesn't access a, b, or c */ END_LOOP out = f(a, c, ...) END Here, a and c must be kept alive for both loops, but b only is needed for a few instructions. However, in a single pass state tracker I have to keep all the information for a, b, and c until the end of the program, because only then I can discard the loop information for b, and resolve everything else for a and c. With the approach I am using, i.e. only collecting all the access information in the pass over the program, and resolving the life-times at the end by re-playing the temp-access timeline, the above can be achieved with less hassle, because I don't need to track per temporary information for all the temporaries all the time, instead, I only need to resolve loops and scopes when it is really needed. [snip] To me the algorithm you lined out looks quite complicated, but not too different from what I'm doing when I replay the access time-line of a register. However, with your approach one has to track the state of each variable all the time, information that could be shared otherwise and might not even needed. [snip] > > > > I hope I didn't miss anything, because after all this is > > admittedly subtle stuff. It is, and this is why I think it is better to separate the steps into manageable chunks that can be put under test. (I admit, I'm a fan of test driven development, and for that I also think it is more important to write test cases instead of sketching out algorithms). > > Still, I think this kind of state-machine approach should > > work and allow you to avoid *lots* of allocations and pointer- > > chasing. The allocations I can (and will) get rid of, but I don't see some pointer-chasing as a problem, since it is encapsulated within class methods. I thank you for your comments, but given that my code is working I don't see that re-doing it from scratch is such a good idea. I think refactoring it to eliminate the allocations and covering additional test cases is a better approach. If this makes it possible to move the implementation to be single pass, then I might consider it, but I think tracking all the information for all temporaries all the time is not such a good idea, especially for large shaders that might have 2000+ temporaries before register merging. Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 0/3] [RFC] mesa/st: glsl_to_tgsi: improved temp-reg lifetime estimation
Am Dienstag, den 13.06.2017, 11:07 +0200 schrieb Nicolai Hähnle: > > > I'm curious what you'd suggest for getting rid of allocations anyway. As the refactoring goes I think I will end up with a hybrid approach: In the temporaries I will not keep the full time line, but the important read/write information - just like you suggested. However, I will not resolve the scopes at the end of each loop, but only after the program is fully scanned. For that I need to keep the information of all scopes available and links to the key scopes fro each temporary. Equal to what you pointed out above, I'll need three allocations for this: - the vector for the scopes, - the vector for the temporaries - a stack to handle the scope changes. To not limit the number of scopes and the scope nesting level the scope vector and the stack might do re-allocations though. I think right now I will not go for tracking whether a temporary is written in both if/else branches or all switch cases. What I want to achieve is that the drivers don't get into trouble because too many temporaries need to be allocated when the TGSI is translated into byte code (test case R600 with 124 free usable registers), and so far this seems to work without tackling this detail. Thank you again for your insights, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v4 0/2] build system: Unify c++11 detection and used [was: configure+mesa/st:check -std=c++11 support and enable tests accordingly]
Am Montag, den 16.10.2017, 10:31 -0400 schrieb Chuck Atkins: > Hi Gert, Emil, > > > I think that using the -std=c++11 check should be good, > > A few things to consider, neither of which is a deal breaker, I just > want to make sure they're not forgotten about: > #1 -std=c++ will cover many cases, but not all. Many commercial > compilers use a different set of options [...] > #2 This only checks support for language suport, not std library > support. This can be problematic where your compiler doesn't have > it's own standard library (i.e. Intel and PGI). This puts you in a > wierd spot where things like range-for loops work because they're a > language feature but std::unique_ptr is not available because it's an > std lib feature. > Both of these are managable, #1 by looping through possible flags to > test for and #2 by performing a test compile of C++ code using > required library features and the compiler flags accepted by #1. > I would even venture to say they could be accounted for as needed, > rather than dealt with apriori. Well, my second patch set was using an m4 file that did exactly that. It would, however, only run one compile test to check whether the features of the requested standard are available (with or without additional compiler flags), but as Emil pointed out, nobody really wants to maintain custom m4 files (and I myself am also far from an expert, I just grabbed a m4 file that was on the Internet). I also think adding a test for each C++11 feature used in the code is too tedious, regardless of the build system, and it would really need a dedicated maintainer. > The use cases should not be forgotten though. Neither of these are > common on a typical end-user linux desktop but they are the norm on > HPC / SuperComputing systems, where Mesa+llvmpipe or Mesa+swr are > heavily relied on for rendering in cpu-only compute clusters. With that in mind I think the best solution to accommodate these use cases too without adding an m4 file is to add a configure flag that makes it possible to set the c++11 enabling flag manually, and harvest the swr compile check that tests whether __cplusplus >= 201103L with this flag set, and maybe add some standard library feature check. Opinions? Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2] r600/sb: bail out if prepare_alu_group() doesn't find a proper scheduling
It is possible that the optimizer ends up in an infinite loop in post_scheduler::schedule_alu(), because post_scheduler::prepare_alu_group() does not find a proper scheduling. This can be deducted from pending.count() being larger than zero and not getting smaller. This patch works around this problem by signalling this failure so that the optimizers bails out and the un-optimized shader is used. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103142 Signed-off-by: Gert Wollny --- Change w.r.t. v1: - In schedule_alu() if pending.count() == 0 then don't expect that this value is reduced by a call to prepare_alu_group(), instead continue the loop until it is exited by "break". I've added you Vadim as to original author to the CC, maybe you can shed a bit more light on what might be going wrong here, and whether there is an easy real fix instead of just a workaround. best regards, Gert Note: Submitter has no mesa-git write access. src/gallium/drivers/r600/sb/sb_sched.cpp | 43 src/gallium/drivers/r600/sb/sb_sched.h | 8 +++--- 2 files changed, 31 insertions(+), 20 deletions(-) diff --git a/src/gallium/drivers/r600/sb/sb_sched.cpp b/src/gallium/drivers/r600/sb/sb_sched.cpp index 5113b75684..2fbec2f77e 100644 --- a/src/gallium/drivers/r600/sb/sb_sched.cpp +++ b/src/gallium/drivers/r600/sb/sb_sched.cpp @@ -711,22 +711,24 @@ void alu_group_tracker::update_flags(alu_node* n) { } int post_scheduler::run() { - run_on(sh.root); - return 0; + return run_on(sh.root) ? 0 : 1; } -void post_scheduler::run_on(container_node* n) { - +bool post_scheduler::run_on(container_node* n) { + int r = true; for (node_riterator I = n->rbegin(), E = n->rend(); I != E; ++I) { if (I->is_container()) { if (I->subtype == NST_BB) { bb_node* bb = static_cast(*I); - schedule_bb(bb); + r = schedule_bb(bb); } else { - run_on(static_cast(*I)); + r = run_on(static_cast(*I)); } + if (!r) + break; } } + return r; } void post_scheduler::init_uc_val(container_node *c, value *v) { @@ -758,7 +760,7 @@ unsigned post_scheduler::init_ucm(container_node *c, node *n) { return F == ucm.end() ? 0 : F->second; } -void post_scheduler::schedule_bb(bb_node* bb) { +bool post_scheduler::schedule_bb(bb_node* bb) { PSC_DUMP( sblog << "scheduling BB " << bb->id << "\n"; if (!pending.empty()) @@ -791,8 +793,10 @@ void post_scheduler::schedule_bb(bb_node* bb) { if (n->is_alu_clause()) { n->remove(); - process_alu(static_cast(n)); - continue; + bool r = process_alu(static_cast(n)); + if (r) + continue; + return false; } n->remove(); @@ -800,6 +804,7 @@ void post_scheduler::schedule_bb(bb_node* bb) { } this->cur_bb = NULL; + return true; } void post_scheduler::init_regmap() { @@ -933,10 +938,10 @@ void post_scheduler::process_fetch(container_node *c) { cur_bb->push_front(c); } -void post_scheduler::process_alu(container_node *c) { +bool post_scheduler::process_alu(container_node *c) { if (c->empty()) - return; + return true; ucm.clear(); alu.reset(); @@ -973,7 +978,7 @@ void post_scheduler::process_alu(container_node *c) { } } - schedule_alu(c); + return schedule_alu(c); } void post_scheduler::update_local_interferences() { @@ -1135,15 +1140,20 @@ void post_scheduler::emit_clause() { emit_index_registers(); } -void post_scheduler::schedule_alu(container_node *c) { +bool post_scheduler::schedule_alu(container_node *c) { assert(!ready.empty() || !ready_copies.empty()); - while (1) { - + bool improving = true; + int last_pending = pending.count(); + while (improving) { prev_regmap = regmap; - if (!prepare_alu_group()) { + + int new_pending = pending.count(); + improving = (new_pending < last_pending) || (last_pending == 0); + last_pending = new_pending; + if (alu.current_idx[0] || alu.current_idx[1]) { regmap = prev_regmap; emit_clause(); @@ -1186,6 +1196,7 @@ void post_scheduler::schedule_alu(container_node *c) {
[Mesa-dev] [PATCH v2 02/10] mesa/st/tests: unify MockCodeLine* classes
* Merge the classes MockCodeLine and MockCodelineWithSwizzle into one and refactor tests accordingly. * Change memory allocations to use ralloc* interface. Signed-off-by: Gert Wollny --- .../tests/test_glsl_to_tgsi_lifetime.cpp | 481 ++--- 1 file changed, 234 insertions(+), 247 deletions(-) diff --git a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp index d0ac8b1020..80ea19fa80 100644 --- a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp +++ b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp @@ -29,35 +29,50 @@ #include #include +#include +#include using std::vector; using std::pair; using std::make_pair; +using std::transform; +using std::copy; + +/* Use this to make the compiler pick the swizzle constructor below */ +struct SWZ {}; /* A line to describe a TGSI instruction for building mock shaders. */ struct MockCodeline { - MockCodeline(unsigned _op): op(_op) {} - MockCodeline(unsigned _op, const vector& _dst, const vector& _src, const vector&_to): - op(_op), dst(_dst), src(_src), tex_offsets(_to){} - unsigned op; - vector dst; - vector src; - vector tex_offsets; -}; + MockCodeline(unsigned _op): op(_op), max_temp_id(0){} + MockCodeline(unsigned _op, const vector& _dst, const vector& _src, +const vector&_to); + + MockCodeline(unsigned _op, const vector>& _dst, +const vector>& _src, +const vector>&_to, SWZ with_swizzle); + + int get_max_reg_id() const { return max_temp_id;} + + glsl_to_tgsi_instruction *get_codeline() const; + + static void set_mem_ctx(void *ctx); + +private: + st_src_reg create_src_register(int src_idx); + st_src_reg create_src_register(int src_idx, const char *swizzle); + st_src_reg create_src_register(int src_idx, gl_register_file file); + + st_dst_reg create_dst_register(int dst_idx); + st_dst_reg create_dst_register(int dst_idx, int writemask); + st_dst_reg create_dst_register(int dst_idx, gl_register_file file); -/* A line to describe a TGSI instruction with swizzeling and write makss - * for building mock shaders. - */ -struct MockCodelineWithSwizzle { - MockCodelineWithSwizzle(unsigned _op): op(_op) {} - MockCodelineWithSwizzle(unsigned _op, const vector>& _dst, - const vector>& _src, - const vector>&_to): - op(_op), dst(_dst), src(_src), tex_offsets(_to){} unsigned op; - vector> dst; - vector> src; - vector> tex_offsets; + vector dst; + vector src; + vector tex_offsets; + + int max_temp_id; + static void *mem_ctx; }; /* A few constants that will notbe tracked as temporary registers by the @@ -72,22 +87,14 @@ const int out1 = -2; class MockShader { public: - MockShader(const vector& source); - MockShader(const vector& source); - ~MockShader(); - - void free(); + MockShader(const vector& source, void *ctx); exec_list* get_program() const; int get_num_temps() const; + private: - st_src_reg create_src_register(int src_idx); - st_dst_reg create_dst_register(int dst_idx); - st_src_reg create_src_register(int src_idx, const char *swizzle); - st_dst_reg create_dst_register(int dst_idx,int writemask); exec_list* program; int num_temps; - void *mem_ctx; }; using expectation = vector>; @@ -102,7 +109,6 @@ protected: class LifetimeEvaluatorTest : public MesaTestWithMemCtx { protected: void run(const vector& code, const expectation& e); - void run(const vector& code, const expectation& e); private: virtual void check(const vector& result, const expectation& e) = 0; }; @@ -136,21 +142,9 @@ protected: class RegisterLifetimeAndRemappingTest : public RegisterRemappingTest { protected: using RegisterRemappingTest::run; - template - void run(const vector& code, const vector& expect); + void run(const vector& code, const vector& expect); }; -template -void RegisterLifetimeAndRemappingTest::run(const vector& code, - const vector& expect) -{ - MockShader shader(code); - std::vector lt(shader.get_num_temps()); - get_temp_registers_required_lifetimes(mem_ctx, shader.get_program(), - shader.get_num_temps(), <[0]); - this->run(lt, expect); -} - TEST_F(LifetimeEvaluatorExactTest, SimpleMoveAdd) { const vector code = { @@ -831,80 +825,81 @@ TEST_F(LifetimeEvaluatorExactTest, FirstWriteAtferReadInNestedLoop) */ TEST_F(LifetimeEvaluatorExactTest, LoopWithConditionalComponentWrite_X) { - const vector code = { - MockCodelineWithSwizzle(TGSI_OPCODE_BGNLOOP), - MockCodelineWithSwizzle(TGSI_OPCODE_MOV, DST(1, WRITEMASK_Y), SRC(in1, "x"), {}), -
[Mesa-dev] [PATCH v2 01/10] mesa/st/tests: Fix zero-byte allocation leaks
Don't allocate a zero-sized array, when no texture offsets are given. Reviewed-by: Nicolai Hähnle Signed-off-by: Gert Wollny --- .../tests/test_glsl_to_tgsi_lifetime.cpp | 23 +++--- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp index 93f4020ebf..d0ac8b1020 100644 --- a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp +++ b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp @@ -1374,10 +1374,14 @@ MockShader::MockShader(const vector& source): next_instr->dst[k] = create_dst_register(i.dst[k].first, i.dst[k].second); } next_instr->tex_offset_num_offset = i.tex_offsets.size(); - next_instr->tex_offsets = new st_src_reg[i.tex_offsets.size()]; - for (unsigned k = 0; k < i.tex_offsets.size(); ++k) { - next_instr->tex_offsets[k] = create_src_register(i.tex_offsets[k].first, - i.tex_offsets[k].second); + if (next_instr->tex_offset_num_offset > 0) { + next_instr->tex_offsets = new st_src_reg[i.tex_offsets.size()]; + for (unsigned k = 0; k < i.tex_offsets.size(); ++k) { +next_instr->tex_offsets[k] = create_src_register(i.tex_offsets[k].first, + i.tex_offsets[k].second); + } + } else { + next_instr->tex_offsets = nullptr; } program->push_tail(next_instr); } @@ -1407,10 +1411,15 @@ MockShader::MockShader(const vector& source): next_instr->dst[k] = create_dst_register(i.dst[k]); } next_instr->tex_offset_num_offset = i.tex_offsets.size(); - next_instr->tex_offsets = new st_src_reg[i.tex_offsets.size()]; - for (unsigned k = 0; k < i.tex_offsets.size(); ++k) { - next_instr->tex_offsets[k] = create_src_register(i.tex_offsets[k]); + if (next_instr->tex_offset_num_offset > 0) { + next_instr->tex_offsets = new st_src_reg[i.tex_offsets.size()]; + for (unsigned k = 0; k < i.tex_offsets.size(); ++k) { +next_instr->tex_offsets[k] = create_src_register(i.tex_offsets[k]); + } + } else { + next_instr->tex_offsets = nullptr; } + program->push_tail(next_instr); } ++num_temps; -- 2.14.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 05/10] mesa/st/glsl_to_tgsi: Correct debug output for indirect access
For arrays print the array ID, and with indirect access also print the reladdr* registers. The reladdr* registers are always used in the printout, even though the actual code may use an address register. Specifically, a sequence involving src.reladdr = TEMP[2] and src.index=10 that emits the address register loading instruction will be printed like: MOV ADDR[0].x, TEMP[2]. MOV TEMP[3], ARRAY(2)[TEMP[2]. + 10] The reason for this is, that there is currently no indication in the src register on whether the address instruction was or must be emitted. Signed-off-by: Gert Wollny --- .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 120 - 1 file changed, 67 insertions(+), 53 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp index 76c198e165..839dfff078 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp @@ -601,7 +601,7 @@ public: #ifndef NDEBUG /* Function used for debugging. */ -static void dump_instruction(int line, prog_scope *scope, +static void dump_instruction(std::ostream& os, int line, prog_scope *scope, const glsl_to_tgsi_instruction& inst); #endif @@ -647,7 +647,7 @@ get_temp_registers_required_lifetimes(void *mem_ctx, exec_list *instructions, break; } - RENAME_DEBUG(dump_instruction(line, cur_scope, *inst)); + RENAME_DEBUG(dump_instruction(cerr, line, cur_scope, *inst)); switch (inst->op) { case TGSI_OPCODE_BGNLOOP: { @@ -918,8 +918,59 @@ static const char *tgsi_file_names[PROGRAM_FILE_MAX] = { "IMM", "BUF", "MEM", "IMAGE" }; +template +void dump_reg_access(std::ostream& os, const st_reg& reg) +{ + os << tgsi_file_names[reg.file]; + if (reg.file == PROGRAM_ARRAY) + os << "(" << reg.array_id << ")"; + + if (reg.has_index2) { + os << "["; + if (reg.reladdr2) + os << *reg.reladdr2 << "+"; + os << reg.index2D << "]"; + } + + os << "["; + if (reg.reladdr) + os << *reg.reladdr << "+"; + os << reg.index << "]"; +} + +static std::ostream& operator << (std::ostream& os, const st_src_reg& reg) +{ + dump_reg_access(os, reg); + + if (reg.swizzle != SWIZZLE_XYZW) { + os << "."; + for (int idx = 0; idx < 4; ++idx) { + int swz = GET_SWZ(reg.swizzle, idx); + if (swz < 4) { +os << swizzle_txt[swz]; + } + } + } + return os; +} + +static std::ostream& operator << (std::ostream& os, const st_dst_reg& dst) +{ + dump_reg_access(os, dst); + + if (dst.writemask != TGSI_WRITEMASK_XYZW) { + os << "."; + if (dst.writemask & TGSI_WRITEMASK_X) os << "x"; + if (dst.writemask & TGSI_WRITEMASK_Y) os << "y"; + if (dst.writemask & TGSI_WRITEMASK_Z) os << "z"; + if (dst.writemask & TGSI_WRITEMASK_W) os << "w"; + } + + return os; +} + static -void dump_instruction(int line, prog_scope *scope, +void dump_instruction(std::ostream& os, int line, prog_scope *scope, const glsl_to_tgsi_instruction& inst) { const struct tgsi_opcode_info *info = tgsi_get_opcode_info(inst.op); @@ -937,74 +988,37 @@ void dump_instruction(int line, prog_scope *scope, info->opcode == TGSI_OPCODE_ENDSWITCH) --indent; - cerr << setw(4) << line << ": "; + os << setw(4) << line << ": "; for (int i = 0; i < indent; ++i) - cerr << ""; - cerr << tgsi_get_opcode_name(info->opcode) << " "; + os << ""; + os << tgsi_get_opcode_name(info->opcode) << " "; bool has_operators = false; for (unsigned j = 0; j < num_inst_dst_regs(&inst); j++) { has_operators = true; if (j > 0) - cerr << ", "; + os << ", "; - const st_dst_reg& dst = inst.dst[j]; - cerr << tgsi_file_names[dst.file]; + os << inst.dst[j]; - if (dst.file == PROGRAM_ARRAY) - cerr << "(" << dst.array_id << ")"; - - cerr << "[" << dst.index << "]"; - - if (dst.writemask != TGSI_WRITEMASK_XYZW) { - cerr << "."; - if (dst.writemask & TGSI_WRITEMASK_X) cerr << "x"; - if (dst.wri
[Mesa-dev] [PATCH v2 00/10] glsl_to_tgsi: Further improvement of lifetime tracking for register merge
Dear all, this is the updated patch set that adds enhanced tracking of IF/ELSE branches and tracking of reladdr* registers for the register_merge step. So far patches 1 & 5 (now 8) are Reviewed-by: Nicolai Hähnle Changes w.r.t. v1: * patches 2-4(new): As suggested by Nikolai, these patches unify the test classes with respect to the different register inputs (at this point: plain and with swizzle). In addition, some comments are corrected and the used of white spaces in the test cases is made more consistent. * patch 5: correct the debug output for indirect addressing. Nikolai suggested that another patch might be in order to properly propagate the information when and which address register is used, but since st_*_reg is passed through various levels by value, I'd prefer to deal with that in another, dedicated patch series. * patch 6: Further improve the tracking algorithm, and, as requested by Nikolai, rename some variables and add comments to make the algorithm clearer. * patch 7: Add yet more tests. * patch 9: Update the tests to adhere to the new, unified interface. * patch 10 (new): remove the no longer needed assert for the use of address registers in register_merge (I was considering to add this to 8, but since that one was already reviewed ...) many thanks for any comments, Gert -- Submitter has no write access to mesa-git Gert Wollny (10): mesa/st/tests: Fix zero-byte allocation leaks mesa/st/tests: unify MockCodeLine* classes mesa/st/tests: base check of number of registers on opcode info mesa/st/tests: cleanup whitespace usage and correct some comments mesa/st/glsl_to_tgsi: Correct debug output for indirect access mesa/st/glsl_to_tgsi: Add tracking of ifelse writes in register merging mesa/st/tests: Add tests for improved tracking of temporaries mesa/st/glsl_to_tgsi: Add tracking of indirect addressing registers mesa/st/tests: Add tests for lifetime tracking with indirect addressing mesa/st/glsl_to_tgsi: remove now unneeded assert. src/mesa/state_tracker/st_glsl_to_tgsi.cpp |1 - .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 540 +++-- .../tests/test_glsl_to_tgsi_lifetime.cpp | 1276 +++- 3 files changed, 1399 insertions(+), 418 deletions(-) -- 2.14.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 08/10] mesa/st/glsl_to_tgsi: Add tracking of indirect addressing registers
So far indirect addressing was not tracked to estimate the temporary life time, and it was not needed, because code to load the address registers was always emitted eliminating the reladdr* handles in the past glsl-to.tgsi stages. Now, with Mareks patch 9a88580a4b3d allowing any 1D register to be used for addressing n some hardware this changed, and the tracking becomes necessary. Because the registers have no direct indication on whether the reladdr* was already loaded into an address register, the temporaries in reladdr* are always tracked as reads. This may result in a slight over-estimation of the lifetime in the cases when the load to the address register was emitted. Reviewed-by: Nicolai Hähnle Signed-off-by: Gert Wollny --- .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 108 ++--- 1 file changed, 74 insertions(+), 34 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp index 4490a44468..0903f2feba 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp @@ -869,6 +869,69 @@ public: } }; +class access_recorder { +public: + access_recorder(int _ntemps); + ~access_recorder(); + + void record_read(const st_src_reg& src, int line, prog_scope *scope); + void record_write(const st_dst_reg& src, int line, prog_scope *scope); + + void get_required_lifetimes(struct lifetime *lifetimes); +private: + + int ntemps; + temp_access *acc; + +}; + +access_recorder::access_recorder(int _ntemps): + ntemps(_ntemps) +{ + acc = new temp_access[ntemps]; +} + +access_recorder::~access_recorder() +{ + delete[] acc; +} + +void access_recorder::record_read(const st_src_reg& src, int line, + prog_scope *scope) +{ + if (src.file == PROGRAM_TEMPORARY) + acc[src.index].record_read(line, scope, src.swizzle); + + if (src.reladdr) + record_read(*src.reladdr, line, scope); + if (src.reladdr2) + record_read(*src.reladdr2, line, scope); +} + +void access_recorder::record_write(const st_dst_reg& dst, int line, + prog_scope *scope) +{ + if (dst.file == PROGRAM_TEMPORARY) + acc[dst.index].record_write(line, scope, dst.writemask); + + if (dst.reladdr) + record_read(*dst.reladdr, line, scope); + if (dst.reladdr2) + record_read(*dst.reladdr2, line, scope); +} + +void access_recorder::get_required_lifetimes(struct lifetime *lifetimes) +{ + RENAME_DEBUG(cerr << "= lifetimes ==\n"); + for(int i = 0; i < ntemps; ++i) { + RENAME_DEBUG(cerr << setw(4) << i); + lifetimes[i] = acc[i].get_required_lifetime(); + RENAME_DEBUG(cerr << ": [" << lifetimes[i].begin << ", " +<< lifetimes[i].end << "]\n"); + } + RENAME_DEBUG(cerr << "==\n\n"); +} + } #ifndef NDEBUG @@ -889,7 +952,6 @@ get_temp_registers_required_lifetimes(void *mem_ctx, exec_list *instructions, int if_id = 1; int switch_id = 0; bool is_at_end = false; - bool ok = true; int n_scopes = 1; /* Count scopes to allocate the needed space without the need for @@ -907,7 +969,8 @@ get_temp_registers_required_lifetimes(void *mem_ctx, exec_list *instructions, } prog_scope_storage scopes(mem_ctx, n_scopes); - temp_access *acc = new temp_access[ntemps]; + + access_recorder access(ntemps); prog_scope *cur_scope = scopes.create(nullptr, outer_scope, 0, 0, line); @@ -936,9 +999,7 @@ get_temp_registers_required_lifetimes(void *mem_ctx, exec_list *instructions, case TGSI_OPCODE_IF: case TGSI_OPCODE_UIF: { assert(num_inst_src_regs(inst) == 1); - const st_src_reg& src = inst->src[0]; - if (src.file == PROGRAM_TEMPORARY) -acc[src.index].record_read(line, cur_scope, src.swizzle); + access.record_read(inst->src[0], line, cur_scope); cur_scope = scopes.create(cur_scope, if_branch, if_id++, cur_scope->nesting_depth() + 1, line + 1); break; @@ -964,14 +1025,12 @@ get_temp_registers_required_lifetimes(void *mem_ctx, exec_list *instructions, } case TGSI_OPCODE_SWITCH: { assert(num_inst_src_regs(inst) == 1); - const st_src_reg& src = inst->src[0]; prog_scope *scope = scopes.create(cur_scope, switch_body, switch_id++, cur_scope->nesting_depth() + 1, line); /* We record the read only for the SWITCH statement itself, like it * is used by the only consumer of TGSI_OPCODE_SWITCH in tgsi_exec.c. */ - if (src.file == PROGRAM_TEMPORARY) -acc[src.index].re
[Mesa-dev] [PATCH v2 04/10] mesa/st/tests: cleanup whitespace usage and correct some comments
Signed-off-by: Gert Wollny --- .../tests/test_glsl_to_tgsi_lifetime.cpp | 127 ++--- 1 file changed, 63 insertions(+), 64 deletions(-) diff --git a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp index 1a3c8cfa32..908791fbf6 100644 --- a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp +++ b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp @@ -289,9 +289,9 @@ TEST_F(LifetimeEvaluatorAtLeastTest, WriteInIfAndElseInLoop) run (code, expectation({{-1,-1}, {0,9}, {3,7}, {7,10}})); } -/* In loop if/else value written in both path, read in else path - * before write and also read later - * - value must survive the whole loop +/* Test that read before write in ELSE path is properly tracked: + * In loop if/else value written in both path but read in else path + * before write and also read later - value must survive the whole loop. */ TEST_F(LifetimeEvaluatorExactTest, WriteInIfAndElseReadInElseInLoop) { @@ -351,9 +351,9 @@ TEST_F(LifetimeEvaluatorExactTest, ReadInLoopInIfBeforeWriteAndLifeToTheEnd) run (code, expectation({{-1,-1}, {0,6}})); } -/* In loop if/else read in one path before written in the same loop - * read after the loop, value must survivethe whole loop and - * to the read. +/* In loop read before written in the same loop read after the loop, + * value must survive the whole loop and to the read. + * This is kind of undefined behaviour though ... */ TEST_F(LifetimeEvaluatorExactTest, ReadInLoopBeforeWriteAndLifeToTheEnd) { @@ -686,7 +686,6 @@ TEST_F(LifetimeEvaluatorExactTest, LoopWithReadWriteInSwitchDifferentCaseFallThr run (code, expectation({{-1,-1}, {0,8}})); } - /* Here we read and write from an to the same temp in the same instruction, * but the read is conditional (select operation), hence the lifetime must * start with the first write. @@ -694,21 +693,21 @@ TEST_F(LifetimeEvaluatorExactTest, LoopWithReadWriteInSwitchDifferentCaseFallThr TEST_F(LifetimeEvaluatorExactTest, WriteSelectFromSelf) { const vector code = { - {TGSI_OPCODE_USEQ, {5}, {in0,in1}, {}}, - {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, - {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, - {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, - {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, - {TGSI_OPCODE_FSLT, {2}, {1,in1}, {}}, - {TGSI_OPCODE_UIF, {}, {2}, {}}, - { TGSI_OPCODE_MOV, {3}, {in1}, {}}, - {TGSI_OPCODE_ELSE}, - { TGSI_OPCODE_MOV, {4}, {in1}, {}}, - { TGSI_OPCODE_MOV, {4}, {4}, {}}, - { TGSI_OPCODE_MOV, {3}, {4}, {}}, - {TGSI_OPCODE_ENDIF}, - {TGSI_OPCODE_MOV, {out1}, {3}, {}}, - {TGSI_OPCODE_END} + { TGSI_OPCODE_USEQ, {5}, {in0,in1}, {}}, + { TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + { TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + { TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + { TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + { TGSI_OPCODE_FSLT, {2}, {1,in1}, {}}, + { TGSI_OPCODE_UIF, {}, {2}, {}}, + { TGSI_OPCODE_MOV, {3}, {in1}, {}}, + { TGSI_OPCODE_ELSE}, + { TGSI_OPCODE_MOV, {4}, {in1}, {}}, + { TGSI_OPCODE_MOV, {4}, {4}, {}}, + { TGSI_OPCODE_MOV, {3}, {4}, {}}, + { TGSI_OPCODE_ENDIF}, + { TGSI_OPCODE_MOV, {out1}, {3}, {}}, + { TGSI_OPCODE_END} }; run (code, expectation({{-1,-1}, {1,5}, {5,6}, {7,13}, {9,11}, {0,4}})); } @@ -1268,21 +1267,21 @@ TEST_F(RegisterRemappingTest, RegisterRemappingMergeZeroLifetimeRegisters) TEST_F(RegisterLifetimeAndRemappingTest, LifetimeAndRemapping) { const vector code = { - {TGSI_OPCODE_USEQ, {5}, {in0,in1}, {}}, - {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, - {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, - {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, - {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, - {TGSI_OPCODE_FSLT, {2}, {1,in1}, {}}, - {TGSI_OPCODE_UIF, {}, {2}, {}}, - { TGSI_OPCODE_MOV, {3}, {in1}, {}}, - {TGSI_OPCODE_ELSE}, - { TGSI_OPCODE_MOV, {4}, {in1}, {}}, - { TGSI_OPCODE_MOV, {4}, {4}, {}}, - { TGSI_OPCODE_MOV, {3}, {4}, {}}, - {TGSI_OPCODE_ENDIF}, - {TGSI_OPCODE_MOV, {out1}, {3}, {}}, - {TGSI_OPCODE_END} + { TGSI_OPCODE_USEQ, {5}, {in0,in1}, {}}, + { TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + { TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + { TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + { TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + { TGSI_OPCODE_FSLT, {2}, {1,in1}, {}}, + { TGSI_OPCODE_UIF, {}, {2}, {}}, + { TGSI_OPCODE_MOV, {3}, {in1}, {}}, + { TGSI_OPCODE_ELSE}, + { TGSI_OPCODE_MOV, {4}, {in1}, {}}, + { TGSI_OPCODE_MOV, {4}, {4}, {}}, + { TGSI_OPCODE_MOV, {3}, {4}, {}}, + { TGSI_OPCODE_ENDIF}, + { TGSI_OPCODE_MOV, {out1}, {3}, {}}, + { TGSI_OPCODE_END} }; run (code, vector({0,1,5,5,1,5})); } @@ -1290,15 +1289,15 @@ TEST_F
[Mesa-dev] [PATCH v2 10/10] mesa/st/glsl_to_tgsi: remove now unneeded assert.
With the implementation of the tracking of the registers used in reladdr asserting that a driver calling merge_register() uses the address register is no longer needed. Signed-off-by: Gert Wollny --- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index a45f0047a8..68b80e015d 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -5257,7 +5257,6 @@ glsl_to_tgsi_visitor::merge_two_dsts(void) void glsl_to_tgsi_visitor::merge_registers(void) { - assert(need_uarl); struct lifetime *lifetimes = rzalloc_array(mem_ctx, struct lifetime, this->next_temp); -- 2.14.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 09/10] mesa/st/tests: Add tests for lifetime tracking with indirect addressing
Add a code line type that accepts one layer of indirect addressing and add tests to check that temporary register access used for indirect addressing is accounted for in the lifetime estimation. Signed-off-by: Gert Wollny --- .../tests/test_glsl_to_tgsi_lifetime.cpp | 184 - 1 file changed, 183 insertions(+), 1 deletion(-) diff --git a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp index 5b6c40ffec..88f8844876 100644 --- a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp +++ b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp @@ -37,10 +37,14 @@ using std::pair; using std::make_pair; using std::transform; using std::copy; +using std::tuple; /* Use this to make the compiler pick the swizzle constructor below */ struct SWZ {}; +/* Use this to make the compiler pick the constructor with reladdr below */ +struct RA {}; + /* A line to describe a TGSI instruction for building mock shaders. */ struct MockCodeline { MockCodeline(unsigned _op): op(_op), max_temp_id(0){} @@ -51,6 +55,10 @@ struct MockCodeline { const vector>& _src, const vector>&_to, SWZ with_swizzle); + MockCodeline(unsigned _op, const vector>& _dst, +const vector>& _src, +const vector>&_to, RA with_reladdr); + int get_max_reg_id() const { return max_temp_id;} glsl_to_tgsi_instruction *get_codeline() const; @@ -61,11 +69,13 @@ private: st_src_reg create_src_register(int src_idx); st_src_reg create_src_register(int src_idx, const char *swizzle); st_src_reg create_src_register(int src_idx, gl_register_file file); + st_src_reg create_src_register(const tuple& src); + st_src_reg *create_rel_src_register(int idx); st_dst_reg create_dst_register(int dst_idx); st_dst_reg create_dst_register(int dst_idx, int writemask); st_dst_reg create_dst_register(int dst_idx, gl_register_file file); - + st_dst_reg create_dst_register(const tuple& dest); unsigned op; vector dst; vector src; @@ -1674,6 +1684,90 @@ TEST_F(LifetimeEvaluatorExactTest, NestedLoopWithWriteAfterBreak) run (code, expectation({{-1,-1}, {0,8}})); } +/* Check lifetime estimation with a relative addressing in src. + * Note, since the lifetime estimation always extends the lifetime + * at to at least one instruction after the last write, for the + * test the last read must be at least two instructions after the + * last write to obtain a proper test. + */ + +TEST_F(LifetimeEvaluatorExactTest, ReadIndirectReladdr1) +{ + const vector code = { + { TGSI_OPCODE_MOV, {1}, {in1}, {}}, + { TGSI_OPCODE_MOV, {2}, {in0}, {}}, + { TGSI_OPCODE_MOV, {{3,0,0}}, {{2,1,0}}, {}, RA()}, + { TGSI_OPCODE_MOV, {out0}, {3}, {}}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,2}, {1,2}, {2,3}})); +} + +/* Check lifetime estimation with a relative addressing in src */ +TEST_F(LifetimeEvaluatorExactTest, ReadIndirectReladdr2) +{ + const vector code = { + { TGSI_OPCODE_MOV , {1}, {in1}, {}}, + { TGSI_OPCODE_MOV , {2}, {in0}, {}}, + { TGSI_OPCODE_MOV , {{3,0,0}}, {{4,0,1}}, {}, RA()}, + { TGSI_OPCODE_MOV , {out0}, {3}, {}}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,2}, {1,2},{2,3}})); +} + +/* Check lifetime estimation with a relative addressing in src */ +TEST_F(LifetimeEvaluatorExactTest, ReadIndirectTexOffsReladdr1) +{ + const vector code = { + { TGSI_OPCODE_MOV , {1}, {in1}, {}}, + { TGSI_OPCODE_MOV , {2}, {in0}, {}}, + { TGSI_OPCODE_MOV , {{3,0,0}}, {{in2,0,0}}, {{5,1,0}}, RA()}, + { TGSI_OPCODE_MOV , {out0}, {3}, {}}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,2}, {1,2}, {2,3}})); +} + +/* Check lifetime estimation with a relative addressing in src */ +TEST_F(LifetimeEvaluatorExactTest, ReadIndirectTexOffsReladdr2) +{ + const vector code = { + { TGSI_OPCODE_MOV , {1}, {in1}, {}}, + { TGSI_OPCODE_MOV , {2}, {in0}, {}}, + { TGSI_OPCODE_MOV , {{3,0,0}}, {{in2,0,0}}, {{2,0,1}}, RA()}, + { TGSI_OPCODE_MOV , {out0}, {3}, {}}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,2}, {1,2}, {2,3}})); +} + +/* Check lifetime estimation with a relative addressing in dst */ +TEST_F(LifetimeEvaluatorExactTest, WriteIndirectReladdr1) +{ + const vector code = { + { TGSI_OPCODE_MOV , {1}, {in0}, {}}, + { TGSI_OPCODE_MOV , {1}, {in1}, {}}, + { TGSI_OPCODE_MOV , {{5,1,0}}, {{in1,0,0}}, {}, RA()}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,2}})); +} + +/* Check lifetime estimation with a relative addressing in dst */ +TEST_F(LifetimeEvaluatorExactTest, WriteIndirectReladdr2) +{ + const vector code = { + { TGSI_OPCODE_MOV , {1}, {in0}, {}}, + { TGSI_OPCODE_MOV , {2}, {in
[Mesa-dev] [PATCH v2 03/10] mesa/st/tests: base check of number of registers on opcode info
* Test number of operands by using num_inst_src_regs/num_inst_dst_regs and fix tests accordingly. Signed-off-by: Gert Wollny --- src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp index 80ea19fa80..1a3c8cfa32 100644 --- a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp +++ b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp @@ -912,7 +912,7 @@ TEST_F(LifetimeEvaluatorExactTest, FRaWSameInstructionInLoopAndCondition) const vector code = { { TGSI_OPCODE_BGNLOOP }, { TGSI_OPCODE_BGNLOOP }, - { TGSI_OPCODE_IF, {0}, {in0}, {} }, + { TGSI_OPCODE_IF, {}, {in0}, {} }, { TGSI_OPCODE_ADD, {1}, {1,in0}, {}}, { TGSI_OPCODE_ENDIF}, { TGSI_OPCODE_MOV, {1}, {in1}, {}}, @@ -1468,8 +1468,8 @@ glsl_to_tgsi_instruction *MockCodeline::get_codeline() const next_instr->op = op; next_instr->info = tgsi_get_opcode_info(op); - assert(src.size() < 5); - assert(dst.size() < 3); + assert(src.size() == num_inst_src_regs(next_instr)); + assert(dst.size() == num_inst_dst_regs(next_instr)); assert(tex_offsets.size() < 3); copy(src.begin(), src.end(), next_instr->src); -- 2.14.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v2 06/10] mesa/st/glsl_to_tgsi: Add tracking of ifelse writes in register merging
Improve the life-time evaluation of temporary registers by also tracking writes in both if and else branches and in up to 32 nested scopes. As a result the estimated required register life-times can be further reduced enabling more registers to be merged. Signed-off-by: Gert Wollny --- .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 312 +++-- 1 file changed, 292 insertions(+), 20 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp index 839dfff078..4490a44468 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp @@ -98,15 +98,19 @@ public: int begin() const; int loop_break_line() const; + const prog_scope *in_else_scope() const; const prog_scope *in_ifelse_scope() const; - const prog_scope *in_switchcase_scope() const; + const prog_scope *in_parent_ifelse_scope() const; const prog_scope *innermost_loop() const; const prog_scope *outermost_loop() const; const prog_scope *enclosing_conditional() const; bool is_loop() const; bool is_in_loop() const; + bool is_switchcase_scope_in_loop() const; bool is_conditional() const; + bool is_child_of(const prog_scope *scope) const; + bool is_child_of_ifelse_id_sibling(const prog_scope *scope) const; bool break_is_for_switchcase() const; bool contains_range_of(const prog_scope& other) const; @@ -137,25 +141,81 @@ private: prog_scope *storage; }; +/* Class to track the access to a component of a temporary register. */ + class temp_comp_access { public: temp_comp_access(); + void record_read(int line, prog_scope *scope); void record_write(int line, prog_scope *scope); lifetime get_required_lifetime(); private: void propagate_lifetime_to_dominant_write_scope(); + bool conditional_ifelse_write_in_loop() const; + + void record_ifelse_write(const prog_scope& scope); + void record_if_write(const prog_scope& scope); + void record_else_write(const prog_scope& scope); prog_scope *last_read_scope; prog_scope *first_read_scope; prog_scope *first_write_scope; + int first_write; int last_read; int last_write; int first_read; - bool keep_for_full_loop; + + /* This member variable tracks the current resolution of conditional writing +* to this temporary in IF/ELSE clauses. +* +* The initial value "conditionality_untouched" indicates that this +* temporary has not yet been written to within an if clause. +* +* A positive (other than "conditionality_untouched") number refers to the +* last loop id for which the write was resolved as unconditional. With each +* new loop this value will be overwitten by "conditionality_unresolved" +* on entering the first IF clause writing this temporary. +* +* The value "conditionality_unresolved" indicates that no resolution has +* been achieved so far. If the variable is set to this value at the end of +* the processing of the whole shader it also indicates a conditional write. +* +* The value "write_is_conditional" marks that the variable is written +* conditionally (i.e. not in all relevant IF/ELSE code path pairs) in at +* least one loop. +*/ + int conditionality_in_loop_id; + + /* Helper constants to make the tracking code more readable. */ + static const int write_is_conditional = -1; + static const int conditionality_unresolved = 0; + static const int conditionality_untouched; + + /* A bit field tracking the nexting levels of if-else clauses where the +* temporary has (so far) been written to in the if branch, but not in the +* else branch. +*/ + unsigned int if_scope_write_flags; + + int next_ifelse_nesting_depth; + static const int supported_ifelse_nesting_depth = 32; + + /* Tracks the last if scope in which the temporary was written to +* without a write in the correspondig else branch. Is also used +* to track read-before-write in the according scope. +*/ + const prog_scope *current_unpaired_if_write_scope; + + /* Flag to resolve read-before-write in the else scope. */ + bool was_written_in_current_else_scope; }; +const int +temp_comp_access::conditionality_untouched = numeric_limits::max(); + +/* Class to track the access to all components of a temporary register. */ class temp_access { public: temp_access(); @@ -258,6 +318,32 @@ const prog_scope *prog_scope::outermost_loop() const return loop; } +bool prog_scope::is_child_of_ifelse_id_sibling(const prog_scope *scope) const +{ + const prog_scope *my_parent = in_parent_ifelse_scope(); + while (my_parent) { + /* is a direct child? */ + if (my_parent == scope) + return false; + /* is a child of the conditions sibling? */ + if (my_parent->id(
[Mesa-dev] [PATCH v2 07/10] mesa/st/tests: Add tests for improved tracking of temporaries
Additional tests are added that check the tracking of access to temporaries in if-else branches. Signed-off-by: Gert Wollny --- .../tests/test_glsl_to_tgsi_lifetime.cpp | 493 - 1 file changed, 486 insertions(+), 7 deletions(-) diff --git a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp index 908791fbf6..5b6c40ffec 100644 --- a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp +++ b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp @@ -270,7 +270,7 @@ TEST_F(LifetimeEvaluatorExactTest, MoveInIfInNestedLoop) * - value must survive from first write to last read in loop * for now we only check that the minimum life time is correct. */ -TEST_F(LifetimeEvaluatorAtLeastTest, WriteInIfAndElseInLoop) +TEST_F(LifetimeEvaluatorExactTest, WriteInIfAndElseInLoop) { const vector code = { { TGSI_OPCODE_MOV, {1}, {in0}, {}}, @@ -312,6 +312,137 @@ TEST_F(LifetimeEvaluatorExactTest, WriteInIfAndElseReadInElseInLoop) run (code, expectation({{-1,-1}, {0,9}, {1,9}, {7,10}})); } + +/* Test that a write in ELSE path only in loop is properly tracked: + * In loop if/else value written in else path and read outside + * - value must survive the whole loop. + */ +TEST_F(LifetimeEvaluatorExactTest, WriteInElseReadInLoop) +{ + const vector code = { + { TGSI_OPCODE_MOV, {1}, {in0}, {}}, + { TGSI_OPCODE_BGNLOOP }, + { TGSI_OPCODE_IF, {}, {1}, {}}, + { TGSI_OPCODE_UADD, {2}, {1,in0}, {}}, + { TGSI_OPCODE_ELSE }, + { TGSI_OPCODE_ADD, {3}, {1,2}, {}}, + { TGSI_OPCODE_ENDIF}, + { TGSI_OPCODE_UADD, {1}, {3,in1}, {}}, + { TGSI_OPCODE_ENDLOOP }, + { TGSI_OPCODE_MOV, {out0}, {1}, {}}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,9}, {1,8}, {1,8}})); +} + +/* Test that tracking a second write in an ELSE path is not attributed + * to the IF path: In loop if/else value written in else path twice and + * read outside - value must survive the whole loop + */ +TEST_F(LifetimeEvaluatorExactTest, WriteInElseTwiceReadInLoop) +{ + const vector code = { + { TGSI_OPCODE_MOV, {1}, {in0}, {}}, + { TGSI_OPCODE_BGNLOOP }, + { TGSI_OPCODE_IF, {}, {1}, {}}, + { TGSI_OPCODE_UADD, {2}, {1,in0}, {}}, + { TGSI_OPCODE_ELSE }, + { TGSI_OPCODE_ADD, {3}, {1,2}, {}}, + { TGSI_OPCODE_ADD, {3}, {1,3}, {}}, + { TGSI_OPCODE_ENDIF}, + { TGSI_OPCODE_UADD, {1}, {3,in1}, {}}, + { TGSI_OPCODE_ENDLOOP }, + { TGSI_OPCODE_MOV, {out0}, {1}, {}}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,10}, {1,9}, {1,9}})); +} + +/* Test that the IF and ELSE scopes from different IF/ELSE pairs are not + * merged: In loop if/else value written in if, and then in different else path + * and read outside - value must survive the whole loop + */ +TEST_F(LifetimeEvaluatorExactTest, WriteInOneIfandInAnotherElseInLoop) +{ + const vector code = { + { TGSI_OPCODE_MOV, {1}, {in0}, {}}, + { TGSI_OPCODE_BGNLOOP }, + { TGSI_OPCODE_IF, {}, {1}, {}}, + { TGSI_OPCODE_UADD, {2}, {1,in0}, {}}, + { TGSI_OPCODE_ENDIF}, + { TGSI_OPCODE_IF, {}, {1}, {}}, + { TGSI_OPCODE_ELSE }, + { TGSI_OPCODE_ADD, {2}, {1,1}, {}}, + { TGSI_OPCODE_ENDIF}, + { TGSI_OPCODE_UADD, {1}, {2,in1}, {}}, + { TGSI_OPCODE_ENDLOOP }, + { TGSI_OPCODE_MOV, {out0}, {1}, {}}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,11}, {1,10}})); +} + +/* Test that with a new loop the resolution of the IF/ELSE write conditionality + * is restarted: In first loop value is written in both if and else, in second + * loop value is written only in if - must survive the second loop. + * However, the tracking is currently not able to restrict the lifetime + * in the first loop, hence the "AtLeast" test. + */ +TEST_F(LifetimeEvaluatorAtLeastTest, UnconditionalInFirstLoopConditionalInSecond) +{ + const vector code = { + { TGSI_OPCODE_MOV, {1}, {in0}, {}}, + { TGSI_OPCODE_BGNLOOP }, + { TGSI_OPCODE_IF, {}, {1}, {}}, + { TGSI_OPCODE_UADD, {2}, {1,in0}, {}}, + { TGSI_OPCODE_ELSE }, + { TGSI_OPCODE_UADD, {2}, {1,in1}, {}}, + { TGSI_OPCODE_ENDIF}, + { TGSI_OPCODE_ENDLOOP }, + { TGSI_OPCODE_BGNLOOP }, + { TGSI_OPCODE_IF, {}, {1}, {}}, + { TGSI_OPCODE_ADD, {2}, {in0,1}, {}}, + { TGSI_OPCODE_ENDIF}, + { TGSI_OPCODE_UADD, {1}, {2,in1}, {}}, + { TGSI_OPCODE_ENDLOOP }, + { TGSI_OPCODE_MOV, {out0}, {1}, {}}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,14}, {3,13}})); +} + +/* Test that with a new loop the resolution of the IF/ELSE write conditionality + * is restarted, and also takes care of write before read in else scope: + * In first loop value is written in both if and else, in s
[Mesa-dev] seems meson vulkan build is currently broken on travis
By testing my own patches I saw that the meson/vulcan specific build failed on travis: https://travis-ci.org/gerddie/mesa/builds/288995180 To check that it is not related to my changes I also did that specific build with the latest git master (35c66f3e4017) that failed. https://travis-ci.org/gerddie/mesa/jobs/289010988 Build log tail: /home/travis/build/gerddie/mesa/_build/../src/amd/vulkan/radv_device.c: 301: undefined reference to `radv_instance_extension_supported' src/amd/vulkan/vulkan_radeon@sha/radv_device.c.o: In function `radv_GetPhysicalDeviceProperties': /home/travis/build/gerddie/mesa/_build/../src/amd/vulkan/radv_device.c: 635: undefined reference to `radv_physical_device_api_version' src/amd/vulkan/vulkan_radeon@sha/radv_device.c.o: In function `radv_CreateDevice': /home/travis/build/gerddie/mesa/_build/../src/amd/vulkan/radv_device.c: 926: undefined reference to `radv_physical_device_extension_supported' collect2: error: ld returned 1 exit status [339/404] Compiling C++ object 'src/intel/compiler/intel_compiler@sta/b rw_vec4_visitor.cpp.o'. ninja: build stopped: subcommand failed. Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] meson: add missing radv_extensions.c generation for libvulkan_radeon
Can't comment much on style, but it fixes the build, so Tested-by: Gert Wollny Am Dienstag, den 17.10.2017, 12:00 +0100 schrieb Eric Engestrom: > Signed-off-by: Eric Engestrom > --- > src/amd/vulkan/meson.build | 10 +- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/src/amd/vulkan/meson.build b/src/amd/vulkan/meson.build > index a5a4f81352807beac92d..6a416d988674504281c6 100644 > --- a/src/amd/vulkan/meson.build > +++ b/src/amd/vulkan/meson.build > @@ -26,6 +26,14 @@ radv_entrypoints = custom_target( > '--outdir', meson.current_build_dir()], > ) > > +radv_extensions = custom_target( > + 'radv_extensions.c', > + input : ['radv_extensions.py', vk_api_xml], > + output : ['radv_extensions.c'], > + command : [prog_python2, '@INPUT0@', '--xml', '@INPUT1@', > + '--out', '@OUTPUT@'], > +) > + > vk_format_table_c = custom_target( >'vk_format_table.c', >input : ['vk_format_table.py', 'vk_format_layout.csv'], > @@ -102,7 +110,7 @@ endif > > libvulkan_radeon = shared_library( >'vulkan_radeon', > - [libradv_files, radv_entrypoints, nir_opcodes_h, > vk_format_table_c], > + [libradv_files, radv_entrypoints, radv_extensions, nir_opcodes_h, > vk_format_table_c], >include_directories : [inc_common, inc_amd, inc_amd_common, > inc_compiler, > inc_vulkan_util, inc_vulkan_wsi], >link_with : [libamd_common, libamdgpu_addrlib, libvulkan_util, ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v5 0/3] build system: Unify c++11 detection and use
Dear all, here is another rework of the c++11 detection and flag application implementing the ideas raised in the discussion with Emil and Chuck. The part in configure.ac has been completely rewritten. Now one can give an environment variable CXX11_CXXFLAGS to test for enabling c++11 support with specific flags. Since in this case one can assume that the users knows what they are doing, this test takes precedence. If CXX11_CXXFLAGS is not given it will be tested whether c++11 is enabled by default (like with g++ >= 6.0) and if not, the flag -std=c++11 is tested. This has the advantage that for g++ >= 6 the whole of mesa will be consistently compiled with the default standard (c++14) instead of forcing one (in that moment older) standard on some parts of the code, while using the newer standard for the rest were it is technically not needed. I dropped the travis patch that added a build with g++4.4 because in the end it only tested whether someone had force-set -std=c++11 somewhere. best regards, Gert -- Submitter has no mesa-git write access. Gert Wollny (3): configure+mesa/st: check for -std=c++11 support and enable mesa/st/test accordingly swr: Replace the check for c++11 by the unified version clover: use the unified check for c++11 instead of the gcc version number configure.ac | 67 --- src/gallium/drivers/swr/Makefile.am | 4 +- src/gallium/state_trackers/clover/Makefile.am | 6 +-- src/mesa/state_tracker/tests/Makefile.am | 4 +- 4 files changed, 69 insertions(+), 12 deletions(-) -- 2.14.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v5 3/3] clover: use the unified check for c++11 instead of the gcc version number
So far clover based its test for compiler support on the version of gcc, while in reality support for c++11 is required. This patch replaces the version check by the check unified for all modules that require c++11. --- configure.ac | 4 ++-- src/gallium/state_trackers/clover/Makefile.am | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/configure.ac b/configure.ac index 2f6809a5f3..9db878c719 100644 --- a/configure.ac +++ b/configure.ac @@ -2296,8 +2296,8 @@ if test "x$enable_opencl" = xyes; then AC_MSG_ERROR([cannot enable OpenCL without Gallium]) fi -if test $GCC_VERSION_MAJOR -lt 4 -o $GCC_VERSION_MAJOR -eq 4 -a $GCC_VERSION_MINOR -lt 7; then -AC_MSG_ERROR([gcc >= 4.7 is required to build clover]) +if test "x$HAVE_CXX11" != "xyes"; then + AC_MSG_ERROR([clover requires c++11 support]) fi if test "x$have_libclc" = xno; then diff --git a/src/gallium/state_trackers/clover/Makefile.am b/src/gallium/state_trackers/clover/Makefile.am index 321393536d..7167bc1c5c 100644 --- a/src/gallium/state_trackers/clover/Makefile.am +++ b/src/gallium/state_trackers/clover/Makefile.am @@ -31,14 +31,14 @@ endif noinst_LTLIBRARIES = libclover.la libcltgsi.la libclllvm.la libcltgsi_la_CXXFLAGS = \ - -std=c++11 \ + $(CXX11_CXXFLAGS) \ $(CLOVER_STD_OVERRIDE) \ $(VISIBILITY_CXXFLAGS) libcltgsi_la_SOURCES = $(TGSI_SOURCES) libclllvm_la_CXXFLAGS = \ - -std=c++11 \ + $(CXX11_CXXFLAGS) \ $(VISIBILITY_CXXFLAGS) \ $(LLVM_CXXFLAGS) \ $(CLOVER_STD_OVERRIDE) \ @@ -51,7 +51,7 @@ libclllvm_la_CXXFLAGS = \ libclllvm_la_SOURCES = $(LLVM_SOURCES) libclover_la_CXXFLAGS = \ - -std=c++11 \ + $(CXX11_CXXFLAGS) \ $(CLOVER_STD_OVERRIDE) \ $(VISIBILITY_CXXFLAGS) -- 2.14.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v5 2/3] swr: Replace the check for c++11 by the unified version
--- configure.ac| 7 +++ src/gallium/drivers/swr/Makefile.am | 4 ++-- 2 files changed, 5 insertions(+), 6 deletions(-) diff --git a/configure.ac b/configure.ac index b828a45310..2f6809a5f3 100644 --- a/configure.ac +++ b/configure.ac @@ -2583,10 +2583,9 @@ if test -n "$with_gallium_drivers"; then xswr) llvm_require_version $LLVM_REQUIRED_SWR "swr" -swr_require_cxx_feature_flags "C++11" "__cplusplus >= 201103L" \ -",-std=c++11" \ -SWR_CXX11_CXXFLAGS -AC_SUBST([SWR_CXX11_CXXFLAGS]) +if test "x$HAVE_CXX11" != "xyes"; then +AC_MSG_ERROR([swr requires c++11 support]) +fi swr_require_cxx_feature_flags "AVX" "defined(__AVX__)" \ ",-target-cpu=sandybridge,-mavx,-march=core-avx,-tp=sandybridge" \ diff --git a/src/gallium/drivers/swr/Makefile.am b/src/gallium/drivers/swr/Makefile.am index b20f128bd2..c995f1b84a 100644 --- a/src/gallium/drivers/swr/Makefile.am +++ b/src/gallium/drivers/swr/Makefile.am @@ -22,7 +22,7 @@ include Makefile.sources include $(top_srcdir)/src/gallium/Automake.inc -AM_CXXFLAGS = $(GALLIUM_DRIVER_CFLAGS) $(SWR_CXX11_CXXFLAGS) +AM_CXXFLAGS = $(GALLIUM_DRIVER_CFLAGS) $(CXX11_CXXFLAGS) noinst_LTLIBRARIES = libmesaswr.la @@ -39,7 +39,7 @@ COMMON_CXXFLAGS = \ -fno-strict-aliasing \ $(GALLIUM_DRIVER_CFLAGS) \ $(LLVM_CXXFLAGS) \ - $(SWR_CXX11_CXXFLAGS) \ + $(CXX11_CXXFLAGS) \ -I$(builddir)/rasterizer/codegen \ -I$(builddir)/rasterizer/core \ -I$(builddir)/rasterizer/jitter \ -- 2.14.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v5 1/3] configure+mesa/st: check for -std=c++11 support and enable mesa/st/test accordingly
Add a check that tests whether the c++ compiler supports c++11, either by default, by adding the compiler flag -std=c++11, or by adding a compiler flag that the user has specified via the environment variable CXX11_CXXFLAGS. The test only does a very shallow check of c++11 support, i.e. it tests whether the define __cplusplus >= 201103L to confirm language support by the compiler, and it checks whether the header is available to test the availability of the c++11 standard library. A make file conditional HAVE_STD_CXX11 is provided that is used in this patch to to enable the tests in mesa/st if C++11 support is available. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102665 --- configure.ac | 56 src/mesa/state_tracker/tests/Makefile.am | 4 ++- 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index bbabf3bed7..b828a45310 100644 --- a/configure.ac +++ b/configure.ac @@ -111,6 +111,10 @@ dnl Check for progs AC_PROG_CPP AC_PROG_CC AC_PROG_CXX +dnl add this here, so the help for this environmnet variable is close to +dnl other CC/CXX flags related help +AC_ARG_VAR([CXX11_CXXFLAGS], [Compiler flag to enable C++11 support (only needed if not + enabled by default and different from -std=c++11)]) AM_PROG_CC_C_O AM_PROG_AS AX_CHECK_GNU_MAKE @@ -120,6 +124,7 @@ AC_PROG_MKDIR_P AC_SYS_LARGEFILE + LT_PREREQ([2.2]) LT_INIT([disable-static]) @@ -327,6 +332,56 @@ if test "x$GCC" = xyes; then fi fi +dnl +dnl Check whether C++11 is supported, if the environment variable +dnl CXX11_CXXFLAGS is set it takes precedence. +dnl + +AC_LANG_PUSH([C++]) + +check_cxx11_available() { +output_support=$1 +AC_COMPILE_IFELSE( +[AC_LANG_PROGRAM([ + #if !(__cplusplus >= 201103L) + #error + #endif + #include +]) +], [ + AC_MSG_RESULT(yes) + cxx11_support=yes +], AC_MSG_RESULT(no)) +eval "$output_support=\$cxx11_support" +} + +HAVE_CXX11=no +save_CXXFLAGS="$CXXFLAGS" + +dnl If the user provides a flag to enable c++11, then we test only this +if test "x$CXX11_CXXFLAGS" != "x"; then + CXXFLAGS="$CXXFLAGS $CXX11_CXXFLAGS" + AC_MSG_CHECKING(whether c++11 is enabled by via $CXX11_CXXFLAGS) + check_cxx11_available HAVE_CXX11 +else + dnl test whether c++11 is enabled by default + AC_MSG_CHECKING(whether c++11 is enabled by default) + check_cxx11_available HAVE_CXX11 + + dnl C++11 not enabled by default, test whether -std=c++11 does the job + if test "x$HAVE_CXX11" != "xyes"; then + CXX11_CXXFLAGS=-std=c++11 + CXXFLAGS="$CXXFLAGS $CXX11_CXXFLAGS" + AC_MSG_CHECKING(whether c++11 is enabled by via $CXX11_CXXFLAGS) + check_cxx11_available HAVE_CXX11 + fi +fi + +CXXFLAGS="$save_CXXFLAGS" +AM_CONDITIONAL(HAVE_STD_CXX11, test "x$HAVE_CXX11" = "xyes") +AC_SUBST(CXX11_CXXFLAGS) +AC_LANG_POP([C++]) + dnl even if the compiler appears to support it, using visibility attributes isn't dnl going to do anything useful currently on cygwin apart from emit lots of warnings case "$host_os" in @@ -3115,6 +3170,7 @@ defines=`echo $DEFINES | $SED 's/^ *//;s/ */ /;s/ *$//'` echo "" echo "CFLAGS: $cflags" echo "CXXFLAGS:$cxxflags" +echo "CXX11_CXXFLAGS: $CXX11_CXXFLAGS" echo "LDFLAGS: $ldflags" echo "Macros: $defines" echo "" diff --git a/src/mesa/state_tracker/tests/Makefile.am b/src/mesa/state_tracker/tests/Makefile.am index f32957608e..6c58d36769 100644 --- a/src/mesa/state_tracker/tests/Makefile.am +++ b/src/mesa/state_tracker/tests/Makefile.am @@ -5,7 +5,7 @@ AM_CFLAGS = \ AM_CXXFLAGS = \ $(GALLIUM_DRIVER_CXXFLAGS) \ - -std=c++11 + $(CXX11_CXXFLAGS) AM_CPPFLAGS = \ -I$(top_srcdir)/src/gtest/include \ @@ -15,8 +15,10 @@ AM_CPPFLAGS = \ -I$(top_builddir)/src/compiler/glsl \ $(DEFINES) +if HAVE_STD_CXX11 TESTS = st-renumerate-test check_PROGRAMS = st-renumerate-test +endif st_renumerate_test_SOURCES = \ test_glsl_to_tgsi_lifetime.cpp -- 2.14.2 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] meson: add opt-out of libunwind
Am Dienstag, den 24.10.2017, 16:44 +0200 schrieb Erik Faye-Lund: > > dep_unwind = dependency('libunwind', required : false) > -if dep_unwind.found() > +if dep_unwind.found() and with_libunwind > pre_args += '-DHAVE_LIBUNWIND' > endif Wouldn't it be better to do something like <<< if with_libunwind dep_unwind = dependency('libunwind', required : false) if dep_unwind.found() pre_args += '-DHAVE_LIBUNWIND' endif endif >>> to avoid looking for the library altogether when it is disabled? One could even think about requiring libunwind if it is not explicitely disabled. Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 03/25] threads: update for late C11 changes
Am Dienstag, den 24.10.2017, 18:21 +0100 schrieb Emil Velikov: > > > Right on both counts, as I don't really have a way of testing on > > Windows. > > > > There's an Appveyour integration similar to Travis. One should be > able to select the MSVC/toolchain version, but I'm not versed enough > for examples :-\ > Let's see what Jose will say on the topic. Appveour seems to be pretty streightforward on how to define which MSVC version to use, i.e. there is a line os: Visual Studio 2013 and later one has to give the right number to scons (apparently msvc2013 =12.0), but for the build a prebuild llvm version llvm-3.3.1-msvc2013-mtd.7z is pulled in, and I'm quite confident that linking against this with any version of MSVC that is not "2013" will fail. Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 01/10] mesa/st/tests: Fix zero-byte allocation leaks
Don't allocate a zero-sized array, when no texture offsets are given. Reviewed-by: Nicolai Hähnle Signed-off-by: Gert Wollny --- .../tests/test_glsl_to_tgsi_lifetime.cpp | 23 +++--- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp index 93f4020ebf..d0ac8b1020 100644 --- a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp +++ b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp @@ -1374,10 +1374,14 @@ MockShader::MockShader(const vector& source): next_instr->dst[k] = create_dst_register(i.dst[k].first, i.dst[k].second); } next_instr->tex_offset_num_offset = i.tex_offsets.size(); - next_instr->tex_offsets = new st_src_reg[i.tex_offsets.size()]; - for (unsigned k = 0; k < i.tex_offsets.size(); ++k) { - next_instr->tex_offsets[k] = create_src_register(i.tex_offsets[k].first, - i.tex_offsets[k].second); + if (next_instr->tex_offset_num_offset > 0) { + next_instr->tex_offsets = new st_src_reg[i.tex_offsets.size()]; + for (unsigned k = 0; k < i.tex_offsets.size(); ++k) { +next_instr->tex_offsets[k] = create_src_register(i.tex_offsets[k].first, + i.tex_offsets[k].second); + } + } else { + next_instr->tex_offsets = nullptr; } program->push_tail(next_instr); } @@ -1407,10 +1411,15 @@ MockShader::MockShader(const vector& source): next_instr->dst[k] = create_dst_register(i.dst[k]); } next_instr->tex_offset_num_offset = i.tex_offsets.size(); - next_instr->tex_offsets = new st_src_reg[i.tex_offsets.size()]; - for (unsigned k = 0; k < i.tex_offsets.size(); ++k) { - next_instr->tex_offsets[k] = create_src_register(i.tex_offsets[k]); + if (next_instr->tex_offset_num_offset > 0) { + next_instr->tex_offsets = new st_src_reg[i.tex_offsets.size()]; + for (unsigned k = 0; k < i.tex_offsets.size(); ++k) { +next_instr->tex_offsets[k] = create_src_register(i.tex_offsets[k]); + } + } else { + next_instr->tex_offsets = nullptr; } + program->push_tail(next_instr); } ++num_temps; -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 00/10] glsl_to_tgsi: Further improvement of lifetime tracking for register merge
Dear all, this is a minor update to the patch set that adds enhanced tracking of IF/ELSE branches and tracking of reladdr* registers for the register_merge step. So far patches 1 & 5 (now 8) are Reviewed-by: Nicolai Hähnle Changes w.r.t. v2: * patch 9: make the creation of register description tuples explicit because this is what in c++11 is actually required (This slipped before because it seems that g++-7.2 handles tuple initialization like it was c++17, also with its default setting -std=c++14). v1: * patches 2-4(new): As suggested by Nikolai, these patches unify the test classes with respect to the different register inputs (at this point: plain and with swizzle). In addition, some comments are corrected and the used of white spaces in the test cases is made more consistent. * patch 5: correct the debug output for indirect addressing. Nikolai suggested that another patch might be in order to properly propagate the information when and which address register is used, but since st_*_reg is passed through various levels by value, I'd prefer to deal with that in another, dedicated patch series. * patch 6: Further improve the tracking algorithm, and, as requested by Nikolai, rename some variables and add comments to make the algorithm clearer. * patch 7: Add yet more tests. * patch 9: Update the tests to adhere to the new, unified interface. * patch 10 (new): remove the no longer needed assert for the use of address registers in register_merge (I was considering to add this to 8, but since that one was already reviewed ...) many thanks for any comments, Gert Gert Wollny (10): mesa/st/tests: Fix zero-byte allocation leaks mesa/st/tests: unify MockCodeLine* classes mesa/st/tests: base check of number of registers on opcode info mesa/st/tests: cleanup whitespace usage and correct some comments mesa/st/glsl_to_tgsi: Correct debug output for indirect access mesa/st/glsl_to_tgsi: Add tracking of ifelse writes in register merging mesa/st/tests: Add tests for improved tracking of temporaries mesa/st/glsl_to_tgsi: Add tracking of indirect addressing registers mesa/st/tests: Add tests for lifetime tracking with indirect addressing mesa/st/glsl_to_tgsi: remove now unneeded assert. src/mesa/state_tracker/st_glsl_to_tgsi.cpp |1 - .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 540 +++-- .../tests/test_glsl_to_tgsi_lifetime.cpp | 1278 +++- 3 files changed, 1401 insertions(+), 418 deletions(-) -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 03/10] mesa/st/tests: base check of number of registers on opcode info
* Test number of operands by using num_inst_src_regs/num_inst_dst_regs and fix tests accordingly. Signed-off-by: Gert Wollny --- src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp index 80ea19fa80..1a3c8cfa32 100644 --- a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp +++ b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp @@ -912,7 +912,7 @@ TEST_F(LifetimeEvaluatorExactTest, FRaWSameInstructionInLoopAndCondition) const vector code = { { TGSI_OPCODE_BGNLOOP }, { TGSI_OPCODE_BGNLOOP }, - { TGSI_OPCODE_IF, {0}, {in0}, {} }, + { TGSI_OPCODE_IF, {}, {in0}, {} }, { TGSI_OPCODE_ADD, {1}, {1,in0}, {}}, { TGSI_OPCODE_ENDIF}, { TGSI_OPCODE_MOV, {1}, {in1}, {}}, @@ -1468,8 +1468,8 @@ glsl_to_tgsi_instruction *MockCodeline::get_codeline() const next_instr->op = op; next_instr->info = tgsi_get_opcode_info(op); - assert(src.size() < 5); - assert(dst.size() < 3); + assert(src.size() == num_inst_src_regs(next_instr)); + assert(dst.size() == num_inst_dst_regs(next_instr)); assert(tex_offsets.size() < 3); copy(src.begin(), src.end(), next_instr->src); -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 02/10] mesa/st/tests: unify MockCodeLine* classes
* Merge the classes MockCodeLine and MockCodelineWithSwizzle into one and refactor tests accordingly. * Change memory allocations to use ralloc* interface. Signed-off-by: Gert Wollny --- .../tests/test_glsl_to_tgsi_lifetime.cpp | 481 ++--- 1 file changed, 234 insertions(+), 247 deletions(-) diff --git a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp index d0ac8b1020..80ea19fa80 100644 --- a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp +++ b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp @@ -29,35 +29,50 @@ #include #include +#include +#include using std::vector; using std::pair; using std::make_pair; +using std::transform; +using std::copy; + +/* Use this to make the compiler pick the swizzle constructor below */ +struct SWZ {}; /* A line to describe a TGSI instruction for building mock shaders. */ struct MockCodeline { - MockCodeline(unsigned _op): op(_op) {} - MockCodeline(unsigned _op, const vector& _dst, const vector& _src, const vector&_to): - op(_op), dst(_dst), src(_src), tex_offsets(_to){} - unsigned op; - vector dst; - vector src; - vector tex_offsets; -}; + MockCodeline(unsigned _op): op(_op), max_temp_id(0){} + MockCodeline(unsigned _op, const vector& _dst, const vector& _src, +const vector&_to); + + MockCodeline(unsigned _op, const vector>& _dst, +const vector>& _src, +const vector>&_to, SWZ with_swizzle); + + int get_max_reg_id() const { return max_temp_id;} + + glsl_to_tgsi_instruction *get_codeline() const; + + static void set_mem_ctx(void *ctx); + +private: + st_src_reg create_src_register(int src_idx); + st_src_reg create_src_register(int src_idx, const char *swizzle); + st_src_reg create_src_register(int src_idx, gl_register_file file); + + st_dst_reg create_dst_register(int dst_idx); + st_dst_reg create_dst_register(int dst_idx, int writemask); + st_dst_reg create_dst_register(int dst_idx, gl_register_file file); -/* A line to describe a TGSI instruction with swizzeling and write makss - * for building mock shaders. - */ -struct MockCodelineWithSwizzle { - MockCodelineWithSwizzle(unsigned _op): op(_op) {} - MockCodelineWithSwizzle(unsigned _op, const vector>& _dst, - const vector>& _src, - const vector>&_to): - op(_op), dst(_dst), src(_src), tex_offsets(_to){} unsigned op; - vector> dst; - vector> src; - vector> tex_offsets; + vector dst; + vector src; + vector tex_offsets; + + int max_temp_id; + static void *mem_ctx; }; /* A few constants that will notbe tracked as temporary registers by the @@ -72,22 +87,14 @@ const int out1 = -2; class MockShader { public: - MockShader(const vector& source); - MockShader(const vector& source); - ~MockShader(); - - void free(); + MockShader(const vector& source, void *ctx); exec_list* get_program() const; int get_num_temps() const; + private: - st_src_reg create_src_register(int src_idx); - st_dst_reg create_dst_register(int dst_idx); - st_src_reg create_src_register(int src_idx, const char *swizzle); - st_dst_reg create_dst_register(int dst_idx,int writemask); exec_list* program; int num_temps; - void *mem_ctx; }; using expectation = vector>; @@ -102,7 +109,6 @@ protected: class LifetimeEvaluatorTest : public MesaTestWithMemCtx { protected: void run(const vector& code, const expectation& e); - void run(const vector& code, const expectation& e); private: virtual void check(const vector& result, const expectation& e) = 0; }; @@ -136,21 +142,9 @@ protected: class RegisterLifetimeAndRemappingTest : public RegisterRemappingTest { protected: using RegisterRemappingTest::run; - template - void run(const vector& code, const vector& expect); + void run(const vector& code, const vector& expect); }; -template -void RegisterLifetimeAndRemappingTest::run(const vector& code, - const vector& expect) -{ - MockShader shader(code); - std::vector lt(shader.get_num_temps()); - get_temp_registers_required_lifetimes(mem_ctx, shader.get_program(), - shader.get_num_temps(), <[0]); - this->run(lt, expect); -} - TEST_F(LifetimeEvaluatorExactTest, SimpleMoveAdd) { const vector code = { @@ -831,80 +825,81 @@ TEST_F(LifetimeEvaluatorExactTest, FirstWriteAtferReadInNestedLoop) */ TEST_F(LifetimeEvaluatorExactTest, LoopWithConditionalComponentWrite_X) { - const vector code = { - MockCodelineWithSwizzle(TGSI_OPCODE_BGNLOOP), - MockCodelineWithSwizzle(TGSI_OPCODE_MOV, DST(1, WRITEMASK_Y), SRC(in1, "x"), {}), -
[Mesa-dev] [PATCH v3 10/10] mesa/st/glsl_to_tgsi: remove now unneeded assert.
With the implementation of the tracking of the registers used in reladdr asserting that a driver calling merge_register() uses the address register is no longer needed. Signed-off-by: Gert Wollny --- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index a45f0047a8..68b80e015d 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -5257,7 +5257,6 @@ glsl_to_tgsi_visitor::merge_two_dsts(void) void glsl_to_tgsi_visitor::merge_registers(void) { - assert(need_uarl); struct lifetime *lifetimes = rzalloc_array(mem_ctx, struct lifetime, this->next_temp); -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 05/10] mesa/st/glsl_to_tgsi: Correct debug output for indirect access
For arrays print the array ID, and with indirect access also print the reladdr* registers. The reladdr* registers are always used in the printout, even though the actual code may use an address register. Specifically, a sequence involving src.reladdr = TEMP[2] and src.index=10 that emits the address register loading instruction will be printed like: MOV ADDR[0].x, TEMP[2]. MOV TEMP[3], ARRAY(2)[TEMP[2]. + 10] The reason for this is, that there is currently no indication in the src register on whether the address instruction was or must be emitted. Signed-off-by: Gert Wollny --- .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 120 - 1 file changed, 67 insertions(+), 53 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp index 76c198e165..839dfff078 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp @@ -601,7 +601,7 @@ public: #ifndef NDEBUG /* Function used for debugging. */ -static void dump_instruction(int line, prog_scope *scope, +static void dump_instruction(std::ostream& os, int line, prog_scope *scope, const glsl_to_tgsi_instruction& inst); #endif @@ -647,7 +647,7 @@ get_temp_registers_required_lifetimes(void *mem_ctx, exec_list *instructions, break; } - RENAME_DEBUG(dump_instruction(line, cur_scope, *inst)); + RENAME_DEBUG(dump_instruction(cerr, line, cur_scope, *inst)); switch (inst->op) { case TGSI_OPCODE_BGNLOOP: { @@ -918,8 +918,59 @@ static const char *tgsi_file_names[PROGRAM_FILE_MAX] = { "IMM", "BUF", "MEM", "IMAGE" }; +template +void dump_reg_access(std::ostream& os, const st_reg& reg) +{ + os << tgsi_file_names[reg.file]; + if (reg.file == PROGRAM_ARRAY) + os << "(" << reg.array_id << ")"; + + if (reg.has_index2) { + os << "["; + if (reg.reladdr2) + os << *reg.reladdr2 << "+"; + os << reg.index2D << "]"; + } + + os << "["; + if (reg.reladdr) + os << *reg.reladdr << "+"; + os << reg.index << "]"; +} + +static std::ostream& operator << (std::ostream& os, const st_src_reg& reg) +{ + dump_reg_access(os, reg); + + if (reg.swizzle != SWIZZLE_XYZW) { + os << "."; + for (int idx = 0; idx < 4; ++idx) { + int swz = GET_SWZ(reg.swizzle, idx); + if (swz < 4) { +os << swizzle_txt[swz]; + } + } + } + return os; +} + +static std::ostream& operator << (std::ostream& os, const st_dst_reg& dst) +{ + dump_reg_access(os, dst); + + if (dst.writemask != TGSI_WRITEMASK_XYZW) { + os << "."; + if (dst.writemask & TGSI_WRITEMASK_X) os << "x"; + if (dst.writemask & TGSI_WRITEMASK_Y) os << "y"; + if (dst.writemask & TGSI_WRITEMASK_Z) os << "z"; + if (dst.writemask & TGSI_WRITEMASK_W) os << "w"; + } + + return os; +} + static -void dump_instruction(int line, prog_scope *scope, +void dump_instruction(std::ostream& os, int line, prog_scope *scope, const glsl_to_tgsi_instruction& inst) { const struct tgsi_opcode_info *info = tgsi_get_opcode_info(inst.op); @@ -937,74 +988,37 @@ void dump_instruction(int line, prog_scope *scope, info->opcode == TGSI_OPCODE_ENDSWITCH) --indent; - cerr << setw(4) << line << ": "; + os << setw(4) << line << ": "; for (int i = 0; i < indent; ++i) - cerr << ""; - cerr << tgsi_get_opcode_name(info->opcode) << " "; + os << ""; + os << tgsi_get_opcode_name(info->opcode) << " "; bool has_operators = false; for (unsigned j = 0; j < num_inst_dst_regs(&inst); j++) { has_operators = true; if (j > 0) - cerr << ", "; + os << ", "; - const st_dst_reg& dst = inst.dst[j]; - cerr << tgsi_file_names[dst.file]; + os << inst.dst[j]; - if (dst.file == PROGRAM_ARRAY) - cerr << "(" << dst.array_id << ")"; - - cerr << "[" << dst.index << "]"; - - if (dst.writemask != TGSI_WRITEMASK_XYZW) { - cerr << "."; - if (dst.writemask & TGSI_WRITEMASK_X) cerr << "x"; - if (dst.wri
[Mesa-dev] [PATCH v3 06/10] mesa/st/glsl_to_tgsi: Add tracking of ifelse writes in register merging
Improve the life-time evaluation of temporary registers by also tracking writes in both if and else branches and in up to 32 nested scopes. As a result the estimated required register life-times can be further reduced enabling more registers to be merged. Signed-off-by: Gert Wollny --- .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 312 +++-- 1 file changed, 292 insertions(+), 20 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp index 839dfff078..4490a44468 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp @@ -98,15 +98,19 @@ public: int begin() const; int loop_break_line() const; + const prog_scope *in_else_scope() const; const prog_scope *in_ifelse_scope() const; - const prog_scope *in_switchcase_scope() const; + const prog_scope *in_parent_ifelse_scope() const; const prog_scope *innermost_loop() const; const prog_scope *outermost_loop() const; const prog_scope *enclosing_conditional() const; bool is_loop() const; bool is_in_loop() const; + bool is_switchcase_scope_in_loop() const; bool is_conditional() const; + bool is_child_of(const prog_scope *scope) const; + bool is_child_of_ifelse_id_sibling(const prog_scope *scope) const; bool break_is_for_switchcase() const; bool contains_range_of(const prog_scope& other) const; @@ -137,25 +141,81 @@ private: prog_scope *storage; }; +/* Class to track the access to a component of a temporary register. */ + class temp_comp_access { public: temp_comp_access(); + void record_read(int line, prog_scope *scope); void record_write(int line, prog_scope *scope); lifetime get_required_lifetime(); private: void propagate_lifetime_to_dominant_write_scope(); + bool conditional_ifelse_write_in_loop() const; + + void record_ifelse_write(const prog_scope& scope); + void record_if_write(const prog_scope& scope); + void record_else_write(const prog_scope& scope); prog_scope *last_read_scope; prog_scope *first_read_scope; prog_scope *first_write_scope; + int first_write; int last_read; int last_write; int first_read; - bool keep_for_full_loop; + + /* This member variable tracks the current resolution of conditional writing +* to this temporary in IF/ELSE clauses. +* +* The initial value "conditionality_untouched" indicates that this +* temporary has not yet been written to within an if clause. +* +* A positive (other than "conditionality_untouched") number refers to the +* last loop id for which the write was resolved as unconditional. With each +* new loop this value will be overwitten by "conditionality_unresolved" +* on entering the first IF clause writing this temporary. +* +* The value "conditionality_unresolved" indicates that no resolution has +* been achieved so far. If the variable is set to this value at the end of +* the processing of the whole shader it also indicates a conditional write. +* +* The value "write_is_conditional" marks that the variable is written +* conditionally (i.e. not in all relevant IF/ELSE code path pairs) in at +* least one loop. +*/ + int conditionality_in_loop_id; + + /* Helper constants to make the tracking code more readable. */ + static const int write_is_conditional = -1; + static const int conditionality_unresolved = 0; + static const int conditionality_untouched; + + /* A bit field tracking the nexting levels of if-else clauses where the +* temporary has (so far) been written to in the if branch, but not in the +* else branch. +*/ + unsigned int if_scope_write_flags; + + int next_ifelse_nesting_depth; + static const int supported_ifelse_nesting_depth = 32; + + /* Tracks the last if scope in which the temporary was written to +* without a write in the correspondig else branch. Is also used +* to track read-before-write in the according scope. +*/ + const prog_scope *current_unpaired_if_write_scope; + + /* Flag to resolve read-before-write in the else scope. */ + bool was_written_in_current_else_scope; }; +const int +temp_comp_access::conditionality_untouched = numeric_limits::max(); + +/* Class to track the access to all components of a temporary register. */ class temp_access { public: temp_access(); @@ -258,6 +318,32 @@ const prog_scope *prog_scope::outermost_loop() const return loop; } +bool prog_scope::is_child_of_ifelse_id_sibling(const prog_scope *scope) const +{ + const prog_scope *my_parent = in_parent_ifelse_scope(); + while (my_parent) { + /* is a direct child? */ + if (my_parent == scope) + return false; + /* is a child of the conditions sibling? */ + if (my_parent->id(
[Mesa-dev] [PATCH v3 09/10] mesa/st/tests: Add tests for lifetime tracking with indirect addressing
Add a code line type that accepts one layer of indirect addressing and add tests to check that temporary register access used for indirect addressing is accounted for in the lifetime estimation. Signed-off-by: Gert Wollny --- .../tests/test_glsl_to_tgsi_lifetime.cpp | 186 - 1 file changed, 185 insertions(+), 1 deletion(-) diff --git a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp index 5b6c40ffec..dbdd4208d8 100644 --- a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp +++ b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp @@ -37,10 +37,14 @@ using std::pair; using std::make_pair; using std::transform; using std::copy; +using std::tuple; /* Use this to make the compiler pick the swizzle constructor below */ struct SWZ {}; +/* Use this to make the compiler pick the constructor with reladdr below */ +struct RA {}; + /* A line to describe a TGSI instruction for building mock shaders. */ struct MockCodeline { MockCodeline(unsigned _op): op(_op), max_temp_id(0){} @@ -51,6 +55,10 @@ struct MockCodeline { const vector>& _src, const vector>&_to, SWZ with_swizzle); + MockCodeline(unsigned _op, const vector>& _dst, +const vector>& _src, +const vector>&_to, RA with_reladdr); + int get_max_reg_id() const { return max_temp_id;} glsl_to_tgsi_instruction *get_codeline() const; @@ -61,11 +69,13 @@ private: st_src_reg create_src_register(int src_idx); st_src_reg create_src_register(int src_idx, const char *swizzle); st_src_reg create_src_register(int src_idx, gl_register_file file); + st_src_reg create_src_register(const tuple& src); + st_src_reg *create_rel_src_register(int idx); st_dst_reg create_dst_register(int dst_idx); st_dst_reg create_dst_register(int dst_idx, int writemask); st_dst_reg create_dst_register(int dst_idx, gl_register_file file); - + st_dst_reg create_dst_register(const tuple& dest); unsigned op; vector dst; vector src; @@ -1674,6 +1684,92 @@ TEST_F(LifetimeEvaluatorExactTest, NestedLoopWithWriteAfterBreak) run (code, expectation({{-1,-1}, {0,8}})); } + +#define MT(X,Y,Z) std::make_tuple(X,Y,Z) +/* Check lifetime estimation with a relative addressing in src. + * Note, since the lifetime estimation always extends the lifetime + * at to at least one instruction after the last write, for the + * test the last read must be at least two instructions after the + * last write to obtain a proper test. + */ + +TEST_F(LifetimeEvaluatorExactTest, ReadIndirectReladdr1) +{ + const vector code = { + { TGSI_OPCODE_MOV, {1}, {in1}, {}}, + { TGSI_OPCODE_MOV, {2}, {in0}, {}}, + { TGSI_OPCODE_MOV, {MT(3,0,0)}, {MT(2,1,0)}, {}, RA()}, + { TGSI_OPCODE_MOV, {out0}, {3}, {}}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,2}, {1,2}, {2,3}})); +} + +/* Check lifetime estimation with a relative addressing in src */ +TEST_F(LifetimeEvaluatorExactTest, ReadIndirectReladdr2) +{ + const vector code = { + { TGSI_OPCODE_MOV , {1}, {in1}, {}}, + { TGSI_OPCODE_MOV , {2}, {in0}, {}}, + { TGSI_OPCODE_MOV , {MT(3,0,0)}, {MT(4,0,1)}, {}, RA()}, + { TGSI_OPCODE_MOV , {out0}, {3}, {}}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,2}, {1,2},{2,3}})); +} + +/* Check lifetime estimation with a relative addressing in src */ +TEST_F(LifetimeEvaluatorExactTest, ReadIndirectTexOffsReladdr1) +{ + const vector code = { + { TGSI_OPCODE_MOV , {1}, {in1}, {}}, + { TGSI_OPCODE_MOV , {2}, {in0}, {}}, + { TGSI_OPCODE_MOV , {MT(3,0,0)}, {MT(in2,0,0)}, {MT(5,1,0)}, RA()}, + { TGSI_OPCODE_MOV , {out0}, {3}, {}}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,2}, {1,2}, {2,3}})); +} + +/* Check lifetime estimation with a relative addressing in src */ +TEST_F(LifetimeEvaluatorExactTest, ReadIndirectTexOffsReladdr2) +{ + const vector code = { + { TGSI_OPCODE_MOV , {1}, {in1}, {}}, + { TGSI_OPCODE_MOV , {2}, {in0}, {}}, + { TGSI_OPCODE_MOV , {MT(3,0,0)}, {MT(in2,0,0)}, {MT(2,0,1)}, RA()}, + { TGSI_OPCODE_MOV , {out0}, {3}, {}}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,2}, {1,2}, {2,3}})); +} + +/* Check lifetime estimation with a relative addressing in dst */ +TEST_F(LifetimeEvaluatorExactTest, WriteIndirectReladdr1) +{ + const vector code = { + { TGSI_OPCODE_MOV , {1}, {in0}, {}}, + { TGSI_OPCODE_MOV , {1}, {in1}, {}}, + { TGSI_OPCODE_MOV , {MT(5,1,0)}, {MT(in1,0,0)}, {}, RA()}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,2}})); +} + +/* Check lifetime estimation with a relative addressing in dst */ +TEST_F(LifetimeEvaluatorExactTest, WriteIndirectReladdr2) +{ + const vector co
[Mesa-dev] [PATCH v3 04/10] mesa/st/tests: cleanup whitespace usage and correct some comments
Signed-off-by: Gert Wollny --- .../tests/test_glsl_to_tgsi_lifetime.cpp | 127 ++--- 1 file changed, 63 insertions(+), 64 deletions(-) diff --git a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp index 1a3c8cfa32..908791fbf6 100644 --- a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp +++ b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp @@ -289,9 +289,9 @@ TEST_F(LifetimeEvaluatorAtLeastTest, WriteInIfAndElseInLoop) run (code, expectation({{-1,-1}, {0,9}, {3,7}, {7,10}})); } -/* In loop if/else value written in both path, read in else path - * before write and also read later - * - value must survive the whole loop +/* Test that read before write in ELSE path is properly tracked: + * In loop if/else value written in both path but read in else path + * before write and also read later - value must survive the whole loop. */ TEST_F(LifetimeEvaluatorExactTest, WriteInIfAndElseReadInElseInLoop) { @@ -351,9 +351,9 @@ TEST_F(LifetimeEvaluatorExactTest, ReadInLoopInIfBeforeWriteAndLifeToTheEnd) run (code, expectation({{-1,-1}, {0,6}})); } -/* In loop if/else read in one path before written in the same loop - * read after the loop, value must survivethe whole loop and - * to the read. +/* In loop read before written in the same loop read after the loop, + * value must survive the whole loop and to the read. + * This is kind of undefined behaviour though ... */ TEST_F(LifetimeEvaluatorExactTest, ReadInLoopBeforeWriteAndLifeToTheEnd) { @@ -686,7 +686,6 @@ TEST_F(LifetimeEvaluatorExactTest, LoopWithReadWriteInSwitchDifferentCaseFallThr run (code, expectation({{-1,-1}, {0,8}})); } - /* Here we read and write from an to the same temp in the same instruction, * but the read is conditional (select operation), hence the lifetime must * start with the first write. @@ -694,21 +693,21 @@ TEST_F(LifetimeEvaluatorExactTest, LoopWithReadWriteInSwitchDifferentCaseFallThr TEST_F(LifetimeEvaluatorExactTest, WriteSelectFromSelf) { const vector code = { - {TGSI_OPCODE_USEQ, {5}, {in0,in1}, {}}, - {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, - {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, - {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, - {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, - {TGSI_OPCODE_FSLT, {2}, {1,in1}, {}}, - {TGSI_OPCODE_UIF, {}, {2}, {}}, - { TGSI_OPCODE_MOV, {3}, {in1}, {}}, - {TGSI_OPCODE_ELSE}, - { TGSI_OPCODE_MOV, {4}, {in1}, {}}, - { TGSI_OPCODE_MOV, {4}, {4}, {}}, - { TGSI_OPCODE_MOV, {3}, {4}, {}}, - {TGSI_OPCODE_ENDIF}, - {TGSI_OPCODE_MOV, {out1}, {3}, {}}, - {TGSI_OPCODE_END} + { TGSI_OPCODE_USEQ, {5}, {in0,in1}, {}}, + { TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + { TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + { TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + { TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + { TGSI_OPCODE_FSLT, {2}, {1,in1}, {}}, + { TGSI_OPCODE_UIF, {}, {2}, {}}, + { TGSI_OPCODE_MOV, {3}, {in1}, {}}, + { TGSI_OPCODE_ELSE}, + { TGSI_OPCODE_MOV, {4}, {in1}, {}}, + { TGSI_OPCODE_MOV, {4}, {4}, {}}, + { TGSI_OPCODE_MOV, {3}, {4}, {}}, + { TGSI_OPCODE_ENDIF}, + { TGSI_OPCODE_MOV, {out1}, {3}, {}}, + { TGSI_OPCODE_END} }; run (code, expectation({{-1,-1}, {1,5}, {5,6}, {7,13}, {9,11}, {0,4}})); } @@ -1268,21 +1267,21 @@ TEST_F(RegisterRemappingTest, RegisterRemappingMergeZeroLifetimeRegisters) TEST_F(RegisterLifetimeAndRemappingTest, LifetimeAndRemapping) { const vector code = { - {TGSI_OPCODE_USEQ, {5}, {in0,in1}, {}}, - {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, - {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, - {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, - {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, - {TGSI_OPCODE_FSLT, {2}, {1,in1}, {}}, - {TGSI_OPCODE_UIF, {}, {2}, {}}, - { TGSI_OPCODE_MOV, {3}, {in1}, {}}, - {TGSI_OPCODE_ELSE}, - { TGSI_OPCODE_MOV, {4}, {in1}, {}}, - { TGSI_OPCODE_MOV, {4}, {4}, {}}, - { TGSI_OPCODE_MOV, {3}, {4}, {}}, - {TGSI_OPCODE_ENDIF}, - {TGSI_OPCODE_MOV, {out1}, {3}, {}}, - {TGSI_OPCODE_END} + { TGSI_OPCODE_USEQ, {5}, {in0,in1}, {}}, + { TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + { TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + { TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + { TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + { TGSI_OPCODE_FSLT, {2}, {1,in1}, {}}, + { TGSI_OPCODE_UIF, {}, {2}, {}}, + { TGSI_OPCODE_MOV, {3}, {in1}, {}}, + { TGSI_OPCODE_ELSE}, + { TGSI_OPCODE_MOV, {4}, {in1}, {}}, + { TGSI_OPCODE_MOV, {4}, {4}, {}}, + { TGSI_OPCODE_MOV, {3}, {4}, {}}, + { TGSI_OPCODE_ENDIF}, + { TGSI_OPCODE_MOV, {out1}, {3}, {}}, + { TGSI_OPCODE_END} }; run (code, vector({0,1,5,5,1,5})); } @@ -1290,15 +1289,15 @@ TEST_F
[Mesa-dev] [PATCH v3 07/10] mesa/st/tests: Add tests for improved tracking of temporaries
Additional teste are added that check the tracking of access to temporaries in if-else branches. Signed-off-by: Gert Wollny --- .../tests/test_glsl_to_tgsi_lifetime.cpp | 493 - 1 file changed, 486 insertions(+), 7 deletions(-) diff --git a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp index 908791fbf6..5b6c40ffec 100644 --- a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp +++ b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp @@ -270,7 +270,7 @@ TEST_F(LifetimeEvaluatorExactTest, MoveInIfInNestedLoop) * - value must survive from first write to last read in loop * for now we only check that the minimum life time is correct. */ -TEST_F(LifetimeEvaluatorAtLeastTest, WriteInIfAndElseInLoop) +TEST_F(LifetimeEvaluatorExactTest, WriteInIfAndElseInLoop) { const vector code = { { TGSI_OPCODE_MOV, {1}, {in0}, {}}, @@ -312,6 +312,137 @@ TEST_F(LifetimeEvaluatorExactTest, WriteInIfAndElseReadInElseInLoop) run (code, expectation({{-1,-1}, {0,9}, {1,9}, {7,10}})); } + +/* Test that a write in ELSE path only in loop is properly tracked: + * In loop if/else value written in else path and read outside + * - value must survive the whole loop. + */ +TEST_F(LifetimeEvaluatorExactTest, WriteInElseReadInLoop) +{ + const vector code = { + { TGSI_OPCODE_MOV, {1}, {in0}, {}}, + { TGSI_OPCODE_BGNLOOP }, + { TGSI_OPCODE_IF, {}, {1}, {}}, + { TGSI_OPCODE_UADD, {2}, {1,in0}, {}}, + { TGSI_OPCODE_ELSE }, + { TGSI_OPCODE_ADD, {3}, {1,2}, {}}, + { TGSI_OPCODE_ENDIF}, + { TGSI_OPCODE_UADD, {1}, {3,in1}, {}}, + { TGSI_OPCODE_ENDLOOP }, + { TGSI_OPCODE_MOV, {out0}, {1}, {}}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,9}, {1,8}, {1,8}})); +} + +/* Test that tracking a second write in an ELSE path is not attributed + * to the IF path: In loop if/else value written in else path twice and + * read outside - value must survive the whole loop + */ +TEST_F(LifetimeEvaluatorExactTest, WriteInElseTwiceReadInLoop) +{ + const vector code = { + { TGSI_OPCODE_MOV, {1}, {in0}, {}}, + { TGSI_OPCODE_BGNLOOP }, + { TGSI_OPCODE_IF, {}, {1}, {}}, + { TGSI_OPCODE_UADD, {2}, {1,in0}, {}}, + { TGSI_OPCODE_ELSE }, + { TGSI_OPCODE_ADD, {3}, {1,2}, {}}, + { TGSI_OPCODE_ADD, {3}, {1,3}, {}}, + { TGSI_OPCODE_ENDIF}, + { TGSI_OPCODE_UADD, {1}, {3,in1}, {}}, + { TGSI_OPCODE_ENDLOOP }, + { TGSI_OPCODE_MOV, {out0}, {1}, {}}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,10}, {1,9}, {1,9}})); +} + +/* Test that the IF and ELSE scopes from different IF/ELSE pairs are not + * merged: In loop if/else value written in if, and then in different else path + * and read outside - value must survive the whole loop + */ +TEST_F(LifetimeEvaluatorExactTest, WriteInOneIfandInAnotherElseInLoop) +{ + const vector code = { + { TGSI_OPCODE_MOV, {1}, {in0}, {}}, + { TGSI_OPCODE_BGNLOOP }, + { TGSI_OPCODE_IF, {}, {1}, {}}, + { TGSI_OPCODE_UADD, {2}, {1,in0}, {}}, + { TGSI_OPCODE_ENDIF}, + { TGSI_OPCODE_IF, {}, {1}, {}}, + { TGSI_OPCODE_ELSE }, + { TGSI_OPCODE_ADD, {2}, {1,1}, {}}, + { TGSI_OPCODE_ENDIF}, + { TGSI_OPCODE_UADD, {1}, {2,in1}, {}}, + { TGSI_OPCODE_ENDLOOP }, + { TGSI_OPCODE_MOV, {out0}, {1}, {}}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,11}, {1,10}})); +} + +/* Test that with a new loop the resolution of the IF/ELSE write conditionality + * is restarted: In first loop value is written in both if and else, in second + * loop value is written only in if - must survive the second loop. + * However, the tracking is currently not able to restrict the lifetime + * in the first loop, hence the "AtLeast" test. + */ +TEST_F(LifetimeEvaluatorAtLeastTest, UnconditionalInFirstLoopConditionalInSecond) +{ + const vector code = { + { TGSI_OPCODE_MOV, {1}, {in0}, {}}, + { TGSI_OPCODE_BGNLOOP }, + { TGSI_OPCODE_IF, {}, {1}, {}}, + { TGSI_OPCODE_UADD, {2}, {1,in0}, {}}, + { TGSI_OPCODE_ELSE }, + { TGSI_OPCODE_UADD, {2}, {1,in1}, {}}, + { TGSI_OPCODE_ENDIF}, + { TGSI_OPCODE_ENDLOOP }, + { TGSI_OPCODE_BGNLOOP }, + { TGSI_OPCODE_IF, {}, {1}, {}}, + { TGSI_OPCODE_ADD, {2}, {in0,1}, {}}, + { TGSI_OPCODE_ENDIF}, + { TGSI_OPCODE_UADD, {1}, {2,in1}, {}}, + { TGSI_OPCODE_ENDLOOP }, + { TGSI_OPCODE_MOV, {out0}, {1}, {}}, + { TGSI_OPCODE_END} + }; + run (code, expectation({{-1,-1}, {0,14}, {3,13}})); +} + +/* Test that with a new loop the resolution of the IF/ELSE write conditionality + * is restarted, and also takes care of write before read in else scope: + * In first loop value is written in both if and else, in s
[Mesa-dev] [PATCH v3 08/10] mesa/st/glsl_to_tgsi: Add tracking of indirect addressing registers
So far indirect addressing was not tracked to estimate the temporary life time, and it was not needed, because code to load the address registers was always emitted eliminating the reladdr* handles in the past glsl-to.tgsi stages. Now, with Mareks patch allowing any 1D register to be used for addressing n some hardware this changed, and the tracking becomes necessary. Because the registers have no direct indication on whether the reladdr* was already loaded into an address register, the temporaries in reladdr* are always tracked as reads. This may result in a slight over-estimation of the lifetime in the cases when the load to the address register was emitted. Reviewed-by: Nicolai Hähnle Signed-off-by: Gert Wollny --- .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 108 ++--- 1 file changed, 74 insertions(+), 34 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp index 4490a44468..0903f2feba 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp @@ -869,6 +869,69 @@ public: } }; +class access_recorder { +public: + access_recorder(int _ntemps); + ~access_recorder(); + + void record_read(const st_src_reg& src, int line, prog_scope *scope); + void record_write(const st_dst_reg& src, int line, prog_scope *scope); + + void get_required_lifetimes(struct lifetime *lifetimes); +private: + + int ntemps; + temp_access *acc; + +}; + +access_recorder::access_recorder(int _ntemps): + ntemps(_ntemps) +{ + acc = new temp_access[ntemps]; +} + +access_recorder::~access_recorder() +{ + delete[] acc; +} + +void access_recorder::record_read(const st_src_reg& src, int line, + prog_scope *scope) +{ + if (src.file == PROGRAM_TEMPORARY) + acc[src.index].record_read(line, scope, src.swizzle); + + if (src.reladdr) + record_read(*src.reladdr, line, scope); + if (src.reladdr2) + record_read(*src.reladdr2, line, scope); +} + +void access_recorder::record_write(const st_dst_reg& dst, int line, + prog_scope *scope) +{ + if (dst.file == PROGRAM_TEMPORARY) + acc[dst.index].record_write(line, scope, dst.writemask); + + if (dst.reladdr) + record_read(*dst.reladdr, line, scope); + if (dst.reladdr2) + record_read(*dst.reladdr2, line, scope); +} + +void access_recorder::get_required_lifetimes(struct lifetime *lifetimes) +{ + RENAME_DEBUG(cerr << "= lifetimes ==\n"); + for(int i = 0; i < ntemps; ++i) { + RENAME_DEBUG(cerr << setw(4) << i); + lifetimes[i] = acc[i].get_required_lifetime(); + RENAME_DEBUG(cerr << ": [" << lifetimes[i].begin << ", " +<< lifetimes[i].end << "]\n"); + } + RENAME_DEBUG(cerr << "==\n\n"); +} + } #ifndef NDEBUG @@ -889,7 +952,6 @@ get_temp_registers_required_lifetimes(void *mem_ctx, exec_list *instructions, int if_id = 1; int switch_id = 0; bool is_at_end = false; - bool ok = true; int n_scopes = 1; /* Count scopes to allocate the needed space without the need for @@ -907,7 +969,8 @@ get_temp_registers_required_lifetimes(void *mem_ctx, exec_list *instructions, } prog_scope_storage scopes(mem_ctx, n_scopes); - temp_access *acc = new temp_access[ntemps]; + + access_recorder access(ntemps); prog_scope *cur_scope = scopes.create(nullptr, outer_scope, 0, 0, line); @@ -936,9 +999,7 @@ get_temp_registers_required_lifetimes(void *mem_ctx, exec_list *instructions, case TGSI_OPCODE_IF: case TGSI_OPCODE_UIF: { assert(num_inst_src_regs(inst) == 1); - const st_src_reg& src = inst->src[0]; - if (src.file == PROGRAM_TEMPORARY) -acc[src.index].record_read(line, cur_scope, src.swizzle); + access.record_read(inst->src[0], line, cur_scope); cur_scope = scopes.create(cur_scope, if_branch, if_id++, cur_scope->nesting_depth() + 1, line + 1); break; @@ -964,14 +1025,12 @@ get_temp_registers_required_lifetimes(void *mem_ctx, exec_list *instructions, } case TGSI_OPCODE_SWITCH: { assert(num_inst_src_regs(inst) == 1); - const st_src_reg& src = inst->src[0]; prog_scope *scope = scopes.create(cur_scope, switch_body, switch_id++, cur_scope->nesting_depth() + 1, line); /* We record the read only for the SWITCH statement itself, like it * is used by the only consumer of TGSI_OPCODE_SWITCH in tgsi_exec.c. */ - if (src.file == PROGRAM_TEMPORARY) -acc[src.index].record_read(line, cur_scope, s
Re: [Mesa-dev] [PATCH 03/25] threads: update for late C11 changes
Am Donnerstag, den 26.10.2017, 13:55 +0900 schrieb Jose Fonseca: > > > If there's doubt, I suggest testing Visual Studio with AppVeyor by > pushing the changes as a feature branch to FDO's git -- I believe > that should trigger an AppVeyor build. (Push to a github repos > hooked into Appveyor, depending on what people are more confortable > with.) Since I have AppVeyor set up, I tested it, but I applied only this patch (the series didn't apply on top of 16cfbef44cf0b1969). The build fails though, I guess since the setup is with MSVC 2013. I'll happily trigger another build, if someone points me to an AppVeyor file that uses a later version of MSVC (that would probably require LLVM build with that other version). Build log: https://ci.appveyor.com/project/gerddie/mesa/build/65 lp_context.c c:\projects\mesa\include\c11\threads_win32.h(151) : error C2037: left of 'tv_sec' specifies undefined struct/union 'timespec' c:\projects\mesa\include\c11\threads_win32.h(151) : error C2037: left of 'tv_nsec' specifies undefined struct/union 'timespec' c:\projects\mesa\include\c11\threads_win32.h(151) : warning C4033: 'impl_timespec2msec' must return a value c:\projects\mesa\include\c11\threads_win32.h(389) : error C2065: 'ts' : undeclared identifier c:\projects\mesa\include\c11\threads_win32.h(389) : warning C4047: 'function' : 'const timespec *' differs in levels of indirection from 'int' c:\projects\mesa\include\c11\threads_win32.h(389) : Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] Meson build failing with "wayland-egl.h no such file or directory"
Commit 16cfbef44cf0 (plus some unrelated patch that I actually wanted to test) fails to build: log snippet: <<< [81/409] Compiling C object 'src/egl/wayland/wayland-egl/wayland-egl@sh a/wayland-egl.c.o'. ... ../src/egl/wayland/wayland-egl/wayland-egl.c:33:25: fatal error: wayland-egl.h: No such file or directory #include "wayland-egl.h" >>> complete log: https://travis-ci.org/gerddie/mesa/jobs/293000195 ^ We had this before with the autotools build: https://lists.freedesktop.org/archives/mesa-dev/2017-October/171399.htm l So I guess it's time to apply https://patchwork.freedesktop.org/patch/180356/ Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] wayland-egl: forward declare wl_egl_window* functions
This patch fixes the build error that can currently (=16cfbef44cf0) be seen with meson, so Tested-by: Gert Wollny Am Dienstag, den 03.10.2017, 15:34 +0200 schrieb Tobias Klausmann: > Starting with commit ab0589c6ed ("wayland-egl: remove no longer > needed wayland-client dependency") the wayland-egl.h include was > missing leading to a build failure: > > CC wayland-egl.lo > wayland-egl.c:33:10: fatal error: wayland-egl.h: No such file or > directory > #include "wayland-egl.h" > ^~~ > > This commit forward declares the following 4 functions from wayland- > egl-core.h, > as suggested by Andres Gomez: > * wl_egl_window_resize > * wl_egl_window_create > * wl_egl_window_destroy > * wl_egl_window_get_attached_size > > With this we can drop the wayland-egl.h include! > > Fixes: ab0589c6ed ("wayland-egl: remove no longer needed wayland- > client > dependency") > Signed-off-by: Tobias Klausmann > > Reviewed-by: Emil Velikov > --- > src/egl/wayland/wayland-egl/wayland-egl-backend.h | 16 > > src/egl/wayland/wayland-egl/wayland-egl.c | 1 - > 2 files changed, 16 insertions(+), 1 deletion(-) > > diff --git a/src/egl/wayland/wayland-egl/wayland-egl-backend.h > b/src/egl/wayland/wayland-egl/wayland-egl-backend.h > index 82f025cb5b..4887d34139 100644 > --- a/src/egl/wayland/wayland-egl/wayland-egl-backend.h > +++ b/src/egl/wayland/wayland-egl/wayland-egl-backend.h > @@ -56,6 +56,22 @@ struct wl_egl_window { > struct wl_surface *surface; > }; > > +struct wl_egl_window * > +wl_egl_window_create(struct wl_surface *surface, > + int width, int height); > + > +void > +wl_egl_window_destroy(struct wl_egl_window *egl_window); > + > +void > +wl_egl_window_resize(struct wl_egl_window *egl_window, > + int width, int height, > + int dx, int dy); > + > +void > +wl_egl_window_get_attached_size(struct wl_egl_window *egl_window, > + int *width, int *height); > + > #ifdef __cplusplus > } > #endif > diff --git a/src/egl/wayland/wayland-egl/wayland-egl.c > b/src/egl/wayland/wayland-egl/wayland-egl.c > index e7cea895ec..b70637cb07 100644 > --- a/src/egl/wayland/wayland-egl/wayland-egl.c > +++ b/src/egl/wayland/wayland-egl/wayland-egl.c > @@ -30,7 +30,6 @@ > #include > #include > > -#include "wayland-egl.h" > #include "wayland-egl-backend.h" > > /* GCC visibility */ ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] meson: do not search for needless deps
Am Mittwoch, den 25.10.2017, 10:24 +0200 schrieb Erik Faye-Lund: > If we don't want to use these deps, there's no good reason to search > for them in the first place. This should shave a bit of time for the > initial build. > --- > > This would be a way of dealing with Gert's suggestion. Goes on top > of the previous patch. > > Thoughts? > > meson.build | 20 ++-- > 1 file changed, 14 insertions(+), 6 deletions(-) > > diff --git a/meson.build b/meson.build > index e842bb1652..201956c4c8 100644 > --- a/meson.build > +++ b/meson.build > @@ -666,9 +666,13 @@ if with_glvnd > endif > > # TODO: make this conditional > -dep_valgrind = dependency('valgrind', required : false) > -if dep_valgrind.found() and with_valgrind > - pre_args += '-DHAVE_VALGRIND' > +if with_valgrind > + dep_valgrind = dependency('valgrind', required : false) > + if dep_valgrind.found() > +pre_args += '-DHAVE_VALGRIND' > + endif > +else > + dep_valgrind = [] > endif > > # pthread stubs. Lets not and say we didn't > @@ -681,9 +685,13 @@ dep_selinux = [] > > # TODO: llvm-prefix and llvm-shared-libs > > -dep_unwind = dependency('libunwind', required : false) > -if dep_unwind.found() and with_libunwind > - pre_args += '-DHAVE_LIBUNWIND' > +if with_libunwind > + dep_unwind = dependency('libunwind', required : false) > + if dep_unwind.found() > +pre_args += '-DHAVE_LIBUNWIND' > + endif > +else > + dep_unwind = [] > endif > > # TODO: flags for opengl, gles, dri Obviously, I like this more, but after reviewing what is done with the autotools, I think it would be preferable if meson would do the same i.e. with autotools --enable-libunwind=no -> don't use libunwind --enable-libunwind=yes -> require libunwind and if not given, then use it if available. The "force using libunwind" option is currently missing. I don't know yet how to use meson properly, but I guess one could achieve this by something like (put in the apropriate files): option('with_libunwind', type : 'combo', choices : ['auto', 'yes', 'no']) if with_libunwind != 'no' dep_unwind = dependency('libunwind', required : with_libunwind == 'yes') if dep_unwind.found() pre_args += '-DHAVE_LIBUNWIND' endif else dep_unwind = [] endif Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [RFC PATCH] util/ralloc: make mem_ctx parameter const for all r*alloc functions
r(z)alloc_size and get_header both take a "const void *" for mem_ctx, but most of the other functions take a "void *" and also the "new" operator in DECLARE_ALLOC_CXX_OPERATORS_TEMPLATE. The latter makes it impossible to copy-construct from a constant declared class variable my using its mem_ctx like void f(const someclass *p) { someclass *q = new(p) someclass(*p); ... } and one has to write someclass *q = ralloc(p, someclass); *q = *p; With this patch declare the mem_ctx parameters as "const void *" for all relevant functions so that a more C++ like deep copy of pointers to classes can be used. Signed-off-by: Gert Wollny --- Submitter has no write access to mesa-git. src/util/ralloc.c | 14 +++--- src/util/ralloc.h | 15 --- 2 files changed, 15 insertions(+), 14 deletions(-) diff --git a/src/util/ralloc.c b/src/util/ralloc.c index 42cfa2e391..cd6bbfa9d4 100644 --- a/src/util/ralloc.c +++ b/src/util/ralloc.c @@ -597,7 +597,7 @@ typedef struct linear_size_chunk linear_size_chunk; /* Allocate the linear buffer with its header. */ static linear_header * -create_linear_node(void *ralloc_ctx, unsigned min_size) +create_linear_node(const void *ralloc_ctx, unsigned min_size) { linear_header *node; @@ -622,7 +622,7 @@ create_linear_node(void *ralloc_ctx, unsigned min_size) } void * -linear_alloc_child(void *parent, unsigned size) +linear_alloc_child(const void *parent, unsigned size) { linear_header *first = LINEAR_PARENT_TO_HEADER(parent); linear_header *latest = first->latest; @@ -657,7 +657,7 @@ linear_alloc_child(void *parent, unsigned size) } void * -linear_alloc_parent(void *ralloc_ctx, unsigned size) +linear_alloc_parent(const void *ralloc_ctx, unsigned size) { linear_header *node; @@ -676,7 +676,7 @@ linear_alloc_parent(void *ralloc_ctx, unsigned size) } void * -linear_zalloc_child(void *parent, unsigned size) +linear_zalloc_child(const void *parent, unsigned size) { void *ptr = linear_alloc_child(parent, size); @@ -686,7 +686,7 @@ linear_zalloc_child(void *parent, unsigned size) } void * -linear_zalloc_parent(void *parent, unsigned size) +linear_zalloc_parent(const void *parent, unsigned size) { void *ptr = linear_alloc_parent(parent, size); @@ -717,7 +717,7 @@ linear_free_parent(void *ptr) } void -ralloc_steal_linear_parent(void *new_ralloc_ctx, void *ptr) +ralloc_steal_linear_parent(const void *new_ralloc_ctx, void *ptr) { linear_header *node; @@ -737,7 +737,7 @@ ralloc_steal_linear_parent(void *new_ralloc_ctx, void *ptr) } void * -ralloc_parent_of_linear_parent(void *ptr) +ralloc_parent_of_linear_parent(const void *ptr) { linear_header *node = LINEAR_PARENT_TO_HEADER(ptr); #ifdef DEBUG diff --git a/src/util/ralloc.h b/src/util/ralloc.h index 05ae8f8407..e5c7993eb5 100644 --- a/src/util/ralloc.h +++ b/src/util/ralloc.h @@ -355,6 +355,7 @@ bool ralloc_asprintf_rewrite_tail(char **str, size_t *start, PRINTFLIKE(3, 4); /** + * Rewrite the tail of an existing string, starting at a given index. * * Overwrites the contents of *str starting at \p start with newly formatted @@ -433,7 +434,7 @@ private: \ reinterpret_cast(p)->~TYPE(); \ } \ public: \ - static void* operator new(size_t size, void *mem_ctx) \ + static void* operator new(size_t size, const void *mem_ctx) \ { \ void *p = ALLOC_FUNC(mem_ctx, size); \ assert(p != NULL); \ @@ -474,7 +475,7 @@ public: \ * \param parent parent node of the linear allocator * \param size size to allocate (max 32 bits) */ -void *linear_alloc_child(void *parent, unsigned size); +void *linear_alloc_child(const void *parent, unsigned size); /** * Allocate a parent node that will hold linear buffers. The returned @@ -484,17 +485,17 @@ void *linear_alloc_child(void *parent, unsigned size); * \param ralloc_ctx ralloc context, must not be NULL * \param sizesize to allocate (max 32 bits) */ -void *linear_alloc_parent(void *ralloc_ctx, unsigned size); +void *linear_alloc_parent(const void *ralloc_ctx, unsigned size); /** * Same as linear_alloc_child, but also clears memory. */ -void *linear_zalloc_child(void *parent, unsigned size); +void *linear_zalloc_child(const void *parent, unsigned size); /** * Same as linear_alloc_parent, but also clears memory. */ -void *linear_zalloc_parent(void *ralloc_ctx, unsigned size); +voi
Re: [Mesa-dev] [PATCH v3 00/10] glsl_to_tgsi: Further improvement of lifetime tracking for register merge
Am Donnerstag, den 26.10.2017, 17:28 +0100 schrieb Emil Velikov: > > > .../tests/test_glsl_to_tgsi_lifetime.cpp | 1278 > > +++- > > JFYI you'd want to explicitly undef NDEBUG in the test. > git grep -10 "#undef NDEBUG" - for examples > > Otherwise the asserts will not trigger since they're not around ;-) > Well, these asserts are not testing library code, they just check the sanity of the test setup, i.e. whether the mock shaders use the right number of source and destination registers with respect to the opcodes. With that in mind I don't think that they really need to be around in a release check build. Nevertheless, I will contemplate whether it makes sence to replace them with the Google test ASSERT_EQ. thanks for the pointer anyway, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/9] gallium: add CAPs to support HW atomic counters. (v2)
Am Freitag, den 03.11.2017, 17:24 +1000 schrieb Dave Airlie: > From: Dave Airlie > [...] > diff --git a/src/gallium/drivers/radeonsi/si_pipe.c > b/src/gallium/drivers/radeonsi/si_pipe.c > index 875aff6..39f7b7b 100644 > --- a/src/gallium/drivers/radeonsi/si_pipe.c > +++ b/src/gallium/drivers/radeonsi/si_pipe.c > @@ -786,6 +786,9 @@ static int si_get_shader_param(struct > pipe_screen* pscreen, > /* Unsupported boolean features. */ > case PIPE_SHADER_CAP_SUBROUTINES: > case PIPE_SHADER_CAP_SUPPORTED_IRS: > + case PIPE_SHADER_CAP_TGSI_DFRACEXP_DLDEXP_SUPPORTED: The patch set does not compile because of this, as of d190bfc1ad PIPE_SHADER_CAP_TGSI_DFRACEXP_DLDEXP_SUPPORTED is already handled with "return 1". Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 8/9] r600: add support for hw atomic counters. (v2)
Am Freitag, den 03.11.2017, 17:24 +1000 schrieb Dave Airlie: > > diff --git a/src/gallium/drivers/r600/r600_pipe.c > b/src/gallium/drivers/r600/r600_pipe.c > index d67a22b..434596b 100644 > --- a/src/gallium/drivers/r600/r600_pipe.c > +++ b/src/gallium/drivers/r600/r600_pipe.c > @@ -74,6 +74,8 @@ static void r600_destroy_context(struct > pipe_context *context) > r600_resource_reference(&rctx->dummy_cmask, NULL); > r600_resource_reference(&rctx->dummy_fmask, NULL); > > + if (rctx->append_fence) > + pipe_resource_reference((struct > pipe_resource**)&rctx->append_fence, NULL); > for (sh = 0; sh < PIPE_SHADER_TYPES; sh++) { > rctx->b.b.set_constant_buffer(&rctx->b.b, sh, > R600_BUFFER_INFO_CONST_BUFFER, NULL); > free(rctx->driver_consts[sh].constants); > @@ -186,6 +188,9 @@ static struct pipe_context > *r600_create_context(struct pipe_screen *screen, > rctx->b.family == > CHIP_CAICOS || > rctx->b.family == > CHIP_CAYMAN || > rctx->b.family == > CHIP_ARUBA); > + > + rctx->append_fence = pipe_buffer_create(rctx- > >b.b.screen, PIPE_BIND_CUSTOM, > + PIPE_USAGE_ > DEFAULT, 32); > break; > default: > R600_ERR("Unsupported chip class %d.\n", rctx- > >b.chip_class); > @@ -605,7 +610,15 @@ static int r600_get_shader_param(struct > pipe_screen* pscreen, > case PIPE_SHADER_CAP_LOWER_IF_THRESHOLD: > case PIPE_SHADER_CAP_TGSI_SKIP_MERGE_REGISTERS: > case PIPE_SHADER_CAP_MAX_HW_ATOMIC_COUNTERS: > + if (rscreen->b.family >= CHIP_CEDAR && rscreen- > >has_atomics) > + return 8; > + return 0; I don't think the other cases should fall through. It makes quite a few piglits fail on BARTS because the number of registers is too high if no merging is applied. Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] gallium/r600 hw atomic support (v3)
Am Dienstag, den 07.11.2017, 16:30 +1000 schrieb Dave Airlie: > This is the 3rd submission of the gallium/r600 hw atomic counter > support. > > This is fixes some rebase artifacts, removes the BUFFER decls from > the TGSI, and fixes some indirect crashes in the r600 backend, Well, I still get some crashes, i.e. piglits spec@arb_arrays_of_arrays@execution@atomic_counters@vs-indirect-index spec@arb_arrays_of_arrays@execution@atomic_counters@fs-indirect-index abort with stack smashing reported. Backtrace for vs-indirect-index is __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51 #1 0x754a4439 in __GI_abort () at abort.c:89 #2 0x754e448f in __libc_message (do_abort=do_abort@entry=2, fm t=fmt@entry=0x755e4066 "*** %s ***: %s terminated\n") at ../sysdeps/posix/libc_fatal.c:175 #3 0x75577027 in __GI___fortify_fail (msg=msg@entry=0x755e 404e "stack smashing detected") at fortify_fail.c:30 #4 0x75576fe0 in __stack_chk_fail () at stack_chk_fail.c:28 #5 0x72489822 in r600_draw_vbo (ctx=0x644740, info=) at r600_state_common.c:2102 #6 0x722b1d90 in u_vbuf_draw_vbo (mgr=0x7c9fa0, info=) at util/u_vbuf.c:1142 #7 0x71fabaa7 in st_draw_vbo (ctx=, prims=0x7fffbe10, nr_prims=, ib=0x0, index_bounds_valid=, min_index=, max_index=, tfb_vertcount=0x0, stream=0, indirect=0x0) at state_tracker/st_draw.c:227 #8 0x71f54060 in vbo_draw_arrays (ctx=ctx@entry=0x782a80, mode =mode@entry=5, start=start@entry=0, count=count@entry=4, numInstances=n umInstances@entry=1, baseInstance=baseInstance@entry=0, drawID=0) at vbo/vbo_exec_array.c:486 #9 0x71f54662 in vbo_exec_DrawArrays (mode=5, start=0, count=4) at vbo/vbo_exec_array.c:641 #10 0x71e7970b in _mesa_unmarshal_DrawArrays (ctx=0x782a80, cmd=0x7cae40, cmd=0x7cae40, cmd=0x7cae40) at main/marshal_generated.c:26752 #11 _mesa_unmarshal_dispatch_cmd (ctx=ctx@entry=0x782a80, cmd=cmd@entry =0x7cae40) at main/marshal_generated.c:42627 #12 0x71e2ee9d in glthread_unmarshal_batch (job=0x7cad28, thread_index=) at main/glthread.c:53 #13 0x71e2f18b in _mesa_glthread_finish (ctx=ctx@entry=0x782a80 ) at main/glthread.c:209 #14 0x71e3ab34 in _mesa_marshal_GetIntegerv (pname=3074, params=0x7fffbf84) at main/marshal_generated.c:37092 #15 0x77b08733 in piglit_can_probe_ubyte () at /home/gerddie/src/Freedesktop/piglit/tests/util/piglit-util-gl.c:1087 #16 0x77b0b97c in piglit_probe_rect_rgba (x=x@entry=0, y=y@entr y=0, w=250, h=250, expected=expected@entry=0x7fffc1d0) at /home/gerddie/src/Freedesktop/piglit/tests/util/piglit-util- gl.c:1413 #17 0x0040b40b in piglit_display () at /home/gerddie/src/Freedesktop/piglit/tests/shaders/shader_runner.c:3475 #18 0x77b2acf5 in run_test (gl_fw=, argc=, argv=) at /home/gerddie/src/Freedesktop/piglit/tests/util/piglit- framework-gl/piglit_fbo_framework.c:52 #19 0x77b175c3 in piglit_gl_test_run (argc=2, argv=argv@entry=0 x7fffd6e8, config=config@entry=0x7fffd590) at /home/gerddie/src/Freedesktop/piglit/tests/util/piglit- framework-gl.c:223 #20 0x00405fe7 in main (argc=, argv=0x7fffd6e8) at /home/gerddie/src/Freedesktop/piglit/tests/shaders/shader_runner.c:61 Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] gallium/r600 atomic - v4
Am Donnerstag, den 09.11.2017, 11:54 +1000 schrieb Dave Airlie: > Hopefully last pass, a few fixes in here, patch 5 is the only > outstanding non-reviewed one, I think I've fixed the sparse > buffer binding in it well enough, there is also fix for Gert's > off-by one. I've run the patch series with the additional fix for patch 7 on the pilit shader subset and don't see any crashes related to this anymore. No regressions and all newly enabled piglits pass, hence for the series: Tested-By: Gert Wollny Thanks, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v6 2/6] mesa/st: glsl_to_tgsi: implement new temporary register lifetime tracker
Just a few comments while updating the patch: > One more comment about the debug_log: as written, it will getenv > every time. It would be better to use the approach taken by e.g. > should_clone_nir() in compiler/nir/nir.h. There's no reason to have > a whole class for this. Actually, debug_log is implemented as a singleton, and the constructor that calles getenv is only called once, but yes, there is no need to make it a class. > > > + > > + assert(num_inst_src_regs(inst) == 1); > > + const st_src_reg& src = inst->src[0]; > > + if (src.file == PROGRAM_TEMPORARY) > > +acc[src.index].record_read(line, switch_scope, > > src.swizzle); > > I think this read (and the one of the switch_register) should both be > on the line of the SWITCH statement, so at the beginning of the > switch_scope. > > What you're doing is only more conservative, and I don't mind that > much, except that at least it should be possible to remove the > set_switch_register and friends, since a read of the switch register > need not be recorded for every CASE statement. Since SWITCH currently is not really generated it is difficult to test what byte code would actually be later generated, but I figured that in the worst case the SWITCH would translate to an IF-chain, in which case for each CASE statement one would actually need to read both, the SWITCH value and the CASE value. > The two branches are inconsistent about where the new scope ends. > Overall, I think this could be simplified: > > prog_scope *switch_scope = > cur_scope->type() == switch_body ? cur_scope : cur_scope- > >parent(); > assert(switch_scope->type() == switch_body); > > prog_scope *scope = scopes.create(...); > > if (cur_scope != switch_body && cur_scope->end() == -1) > scope->set_previous_case_scope(cur_scope); > cur_scope = scope; > > For that matter, I'm not sure the previous_case_scope handling is > necessary or whether it's correct at all. Do I don't think it is necessary, and I will check whether it breaks something. > you really need the end to overlap the next scope on fall-through? > And if yes, what happens if have multiple fall-throughs in a row? I think that this case is handled consistently. Anyway, I will contemplate about whether it makes sense to keep this behavior, and I actually lean towards removing it. > + > > +void temp_access::update_access_mask(int mask) > > +{ > > + if (!first_access_mask) { > > + first_access_mask = mask; > > + } else { > > + needs_component_tracking |= (first_access_mask != mask); > > + } > > + summary_access_mask |= mask; > > This can be simplified to: > > if (mask != access_mask) > needs_component_tracking = true; > access_mask |= mask; > > saving one member variable. Actually, it must be if (access_mask && (mask != access_mask)) needs_component_tracking = true; ... otherwise needs_component_tracking would always be set to true. > > > +* the life time to [lr,lr), so it can me merged "away" > > +*/ > > + if (last_write < 0) > > + return make_lifetime(last_read, last_read); > > Ah I see. I think it'd be better to return (-1, -1) here and handle > that case when merging the lifetimes. It's not more code, since it > allows you to merge the first two case in this function, and it > avoids an overly conservative extension of the register lifetime in > the unlikely case where a component is read from outside the scope of > the other components. My thinking was to completely ignore unused registers, and use read- only registers for renaming, but now I checked that a register that is only read is actually eliminated in glsl_to_tgsi_visitor::renumber_registers so no need to bother distinguishing the two cases. Best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v7 2/6] mesa/st: glsl_to_tgsi: implement new temporary register lifetime tracker
This patch adds a class for tracking the life times of temporary registers in the glsl to tgsi translation. The algorithm runs in three steps: First, in order to minimize the number of needed memory allocations the program is scanned to evaluate the number of scopes. Then, the program is scanned second time to record the important register access time points: first and last reads and writes and their link to the execution scope (loop, if/else branch, switch case). In the third step for each register the actual minimal life time is evaluated. In addition, when compiled in debug mode (i.e. NDEBUG is not defined) the shaders and estimated temporary life times can be logged to stderr by setting the environment variable GLSL_TO_TGSI_RENAME_DEBUG. --- src/mesa/Makefile.sources | 2 + .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 881 + .../state_tracker/st_glsl_to_tgsi_temprename.h | 55 ++ 3 files changed, 938 insertions(+) create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_temprename.h diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources index 5ec4e2d9a2..f1effdb8c1 100644 --- a/src/mesa/Makefile.sources +++ b/src/mesa/Makefile.sources @@ -510,6 +510,8 @@ STATETRACKER_FILES = \ state_tracker/st_glsl_to_tgsi.h \ state_tracker/st_glsl_to_tgsi_private.cpp \ state_tracker/st_glsl_to_tgsi_private.h \ + state_tracker/st_glsl_to_tgsi_temprename.cpp \ + state_tracker/st_glsl_to_tgsi_temprename.h \ state_tracker/st_glsl_types.cpp \ state_tracker/st_glsl_types.h \ state_tracker/st_manager.c \ diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp new file mode 100644 index 00..46a707ec74 --- /dev/null +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp @@ -0,0 +1,881 @@ +/* + * Copyright © 2017 Gert Wollny + * + * 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 (including the next + * paragraph) 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 "st_glsl_to_tgsi_temprename.h" +#include +#include +#include +#include +#include + +/* std::sort is significantly faster than qsort */ +#define USE_STL_SORT +#ifdef USE_STL_SORT +#include +#endif + +#ifndef NDEBUG +#include +#include +#include +#include +using std::cerr; +using std::setw; +#endif + +using std::numeric_limits; + +/* Without c++11 define the nullptr for forward-compatibility + * and better readibility */ +#if __cplusplus < 201103L +#define nullptr 0 +#endif + +#ifndef NDEBUG +/* Helper function to check whether we want to seen debugging output */ +static inline bool is_debug_enabled () +{ + static int debug_enabled = -1; + if (debug_enabled < 0) + debug_enabled = env_var_as_boolean("GLSL_TO_TGSI_RENAME_DEBUG", false); + return debug_enabled > 0; +} +#define RENAME_DEBUG(X) if (is_debug_enabled()) do { X; } while (false); +#else +#define RENAME_DEBUG(X) +#endif + +namespace { + +enum prog_scope_type { + outer_scope, /* Outer program scope */ + loop_body, /* Inside a loop */ + if_branch, /* Inside if branch */ + else_branch, /* Inside else branch */ + switch_body, /* Inside switch statmenet */ + switch_case_branch,/* Inside switch case statmenet */ + switch_default_branch, /* Inside switch default statmenet */ + undefined_scope +}; + +class prog_scope { +public: + prog_scope(prog_scope *parent, prog_scope_type type, int id, + int depth, int begin); + + prog_scope_type type() const; + prog_scope *parent() const; + int nesting_depth() const; + int id() const; + int end() const; + int begin() const; + int loop_break_line() const; + + const prog_scope *in_ifelse_scope() const; + const prog_scope *in_switchcase_sc
[Mesa-dev] [PATCH v7 0/6] mesa/st: glsl_to_tgsi: improved temp-reg lifetime estimation
Hello Nicolai, many thanks for the review. With this new patch set I've implemented most of your suggestions, patch 1 and 6 are only rebased though. The patches are also available via https://github.com/gerddie/mesa/tree/regrename-v7 The one thing I did not do (or better said, I reverted it) is using _mesa_register_file_name in the debug output because the names returned are not correct for the TGSI file indices I have access to there. Piglit doesn't show any regressions and the changes are: - Rebase to 9ee67467 - Correct documentation of GLSL_TO_TGSI_RENAME_DEBUG in commit message. - Fix typos, include files, formatting, and some variable names. - Add anonymous namespace around classes. - Replace debug_log singleton by a function. - Cleanup the switch-case scope creation. - Track switch variable only for SWITCH statement. - Add test case with switch in loop that has a case where the register is written, then falls through where it is read and correct code accordingly. Specifically, this showed that you were right that the scope of the case that falls through must not be extended. - Add test case for more then one break in loop and make sure the first break is used for life-time tracking. - Simplify access flag tracking and merging. - Drop special handling of registers that are only read, i.e. treat them like unused registers since renumber_registers will do this anyway. - Simplify the enclosing scope resolution. - Handle TEMP[0] just like the others. Somehow I had the idea that TEMP[0] is special and should not be touched, and the debug output running various programs and also from piglit shaders doesn't show TEMP[0] as used. - Optimize renaming evaluation by only copying used registers to the reg_access array that is used for the evaluation. Best, Gert Gert Wollny (6): mesa/st: glsl_to_tgsi move some helper classes to extra files mesa/st: glsl_to_tgsi: implement new temporary register lifetime tracker mesa/st: glsl_to_tgsi: add tests for the new temporary lifetime tracker mesa/st: glsl_to_tgsi: add register rename mapping evaluator mesa/st: glsl_to_tgsi: Add test set for evaluation of rename mapping mesa/st: glsl_to_tgsi: tie in new temporary register merge approach configure.ac |1 + src/mesa/Makefile.am |2 +- src/mesa/Makefile.sources |4 + src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 344 + src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp | 196 +++ src/mesa/state_tracker/st_glsl_to_tgsi_private.h | 167 ++ .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 992 .../state_tracker/st_glsl_to_tgsi_temprename.h | 67 + src/mesa/state_tracker/tests/Makefile.am | 36 + .../tests/test_glsl_to_tgsi_lifetime.cpp | 1597 10 files changed, 3073 insertions(+), 333 deletions(-) create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_private.h create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_temprename.h create mode 100644 src/mesa/state_tracker/tests/Makefile.am create mode 100644 src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp -- 2.13.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v7 5/6] mesa/st: glsl_to_tgsi: Add test set for evaluation of rename mapping
The patch adds tests for the register rename mapping evaluation and combined life time estimation and renaming. --- .../tests/test_glsl_to_tgsi_lifetime.cpp | 192 + 1 file changed, 192 insertions(+) diff --git a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp index b9dadcc03d..f4820d4777 100644 --- a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp +++ b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp @@ -124,6 +124,33 @@ protected: void check(const vector& result, const expectation& e); }; +/* With this test class the renaming mapping estimation is tested */ +class RegisterRemappingTest : public MesaTestWithMemCtx { +protected: + void run(const vector& lt, const vector& expect); +}; + +/* With this test class the combined lifetime estimation and renaming + * mepping estimation is tested + */ +class RegisterLifetimeAndRemappingTest : public RegisterRemappingTest { +protected: + using RegisterRemappingTest::run; + template + void run(const vector& code, const vector& expect); +}; + +template +void RegisterLifetimeAndRemappingTest::run(const vector& code, + const vector& expect) +{ + MockShader shader(code); + std::vector lt(shader.get_num_temps()); + get_temp_registers_required_lifetimes(mem_ctx, shader.get_program(), + shader.get_num_temps(), <[0]); + this->run(lt, expect); +} + TEST_F(LifetimeEvaluatorExactTest, SimpleMoveAdd) { const vector code = { @@ -1175,6 +1202,148 @@ TEST_F(LifetimeEvaluatorExactTest, NestedLoopWithWriteAfterBreak) run (code, expectation({{-1,-1}, {0,8}})); } +/* Test remapping table of registers. The tests don't assume + * that the sorting algorithm used to sort the lifetimes + * based on their 'begin' is stable. + */ +TEST_F(RegisterRemappingTest, RegisterRemapping1) +{ + vector lt({{-1,-1}, +{0,1}, +{0,2}, +{1,2}, +{2,10}, +{3,5}, +{5,10} + }); + + vector expect({0,1,2,1,1,2,2}); + run(lt, expect); +} + +TEST_F(RegisterRemappingTest, RegisterRemapping2) +{ + vector lt({{-1,-1}, +{0,1}, +{0,2}, +{3,4}, +{4,5}, + }); + vector expect({0,1,2,1,1}); + run(lt, expect); +} + +TEST_F(RegisterRemappingTest, RegisterRemappingMergeAllToOne) +{ + vector lt({{-1,-1}, +{0,1}, +{1,2}, +{2,3}, +{3,4}, + }); + vector expect({0,1,1,1,1}); + run(lt, expect); +} + +TEST_F(RegisterRemappingTest, RegisterRemappingIgnoreUnused) +{ + vector lt({{-1,-1}, +{0,1}, +{1,2}, +{2,3}, +{-1,-1}, +{3,4}, + }); + vector expect({0,1,1,1,4,1}); + run(lt, expect); +} + +TEST_F(RegisterRemappingTest, RegisterRemappingMergeZeroLifetimeRegisters) +{ + vector lt({{-1,-1}, +{0,1}, +{1,2}, +{2,3}, +{3,3}, +{3,4}, + }); + vector expect({0,1,1,1,1,1}); + run(lt, expect); +} + +TEST_F(RegisterLifetimeAndRemappingTest, LifetimeAndRemapping) +{ + const vector code = { + {TGSI_OPCODE_USEQ, {5}, {in0,in1}, {}}, + {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + {TGSI_OPCODE_FSLT, {2}, {1,in1}, {}}, + {TGSI_OPCODE_UIF, {}, {2}, {}}, + { TGSI_OPCODE_MOV, {3}, {in1}, {}}, + {TGSI_OPCODE_ELSE}, + { TGSI_OPCODE_MOV, {4}, {in1}, {}}, + { TGSI_OPCODE_MOV, {4}, {4}, {}}, + { TGSI_OPCODE_MOV, {3}, {4}, {}}, + {TGSI_OPCODE_ENDIF}, + {TGSI_OPCODE_MOV, {out1}, {3}, {}}, + {TGSI_OPCODE_END} + }; + run (code, vector({0,1,5,5,1,5})); +} + +TEST_F(RegisterLifetimeAndRemappingTest, LifetimeAndRemappingWithUnusedReadOnlyIgnored) +{ + const vector code = { + {TGSI_OPCODE_USEQ, {1}, {in0,in1}, {}}, + {TGSI_OPCODE_UCMP, {2}, {1,in1,2}, {}}, + {TGSI_OPCODE_UCMP, {4}, {2,in1,1}, {}}, + {TGSI_OPCODE_ADD, {5}, {2,4}, {}}, + {TGSI_OPCODE_UIF, {}, {7}, {}}, + { TGSI_OPCODE_ADD, {8}, {5,4}, {}}, + {TGSI_OPCODE_ENDIF}, + {TGSI_OPCODE_MOV, {out1}, {8}, {}}, + {TGSI_OPCODE_END} + }; + /* lt: 1: 0-2,2: 1-3 3: u 4: 2-5 5: 3-5 6: u 7: 0-(-1),8: 5-7 */ + run (code, vector({0,1,2,3,1,2,6,7,1})); +} + +TEST_F(RegisterLifetimeAndRemappingTest, LifetimeAndRemap
[Mesa-dev] [PATCH v7 1/6] mesa/st: glsl_to_tgsi move some helper classes to extra files
eg; -}; - struct glsl_to_tgsi_visitor : public ir_visitor { public: glsl_to_tgsi_visitor(); @@ -597,7 +347,7 @@ fail_link(struct gl_shader_program *prog, const char *fmt, ...) prog->data->LinkStatus = linking_failure; } -static int +int swizzle_for_size(int size) { static const int size_swizzles[4] = { @@ -611,40 +361,6 @@ swizzle_for_size(int size) return size_swizzles[size - 1]; } -static bool -is_resource_instruction(unsigned opcode) -{ - switch (opcode) { - case TGSI_OPCODE_RESQ: - case TGSI_OPCODE_LOAD: - case TGSI_OPCODE_ATOMUADD: - case TGSI_OPCODE_ATOMXCHG: - case TGSI_OPCODE_ATOMCAS: - case TGSI_OPCODE_ATOMAND: - case TGSI_OPCODE_ATOMOR: - case TGSI_OPCODE_ATOMXOR: - case TGSI_OPCODE_ATOMUMIN: - case TGSI_OPCODE_ATOMUMAX: - case TGSI_OPCODE_ATOMIMIN: - case TGSI_OPCODE_ATOMIMAX: - return true; - default: - return false; - } -} - -static unsigned -num_inst_dst_regs(const glsl_to_tgsi_instruction *op) -{ - return op->info->num_dst; -} - -static unsigned -num_inst_src_regs(const glsl_to_tgsi_instruction *op) -{ - return op->info->is_tex || is_resource_instruction(op->op) ? - op->info->num_src - 1 : op->info->num_src; -} glsl_to_tgsi_instruction * glsl_to_tgsi_visitor::emit_asm(ir_instruction *ir, unsigned op, diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp new file mode 100644 index 00..0075ae8676 --- /dev/null +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp @@ -0,0 +1,196 @@ +/* + * Copyright © 2010 Intel Corporation + * Copyright © 2011 Bryan Cain + * Copyright © 2017 Gert Wollny + * + * 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 (including the next + * paragraph) 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 "st_glsl_to_tgsi_private.h" +#include +#include + +static int swizzle_for_type(const glsl_type *type, int component = 0) +{ + unsigned num_elements = 4; + + if (type) { + type = type->without_array(); + if (type->is_scalar() || type->is_vector() || type->is_matrix()) + num_elements = type->vector_elements; + } + + int swizzle = swizzle_for_size(num_elements); + assert(num_elements + component <= 4); + + swizzle += component * MAKE_SWIZZLE4(1, 1, 1, 1); + return swizzle; +} + +st_src_reg::st_src_reg(gl_register_file file, int index, const glsl_type *type, + int component, unsigned array_id) +{ + assert(file != PROGRAM_ARRAY || array_id != 0); + this->file = file; + this->index = index; + this->swizzle = swizzle_for_type(type, component); + this->negate = 0; + this->abs = 0; + this->index2D = 0; + this->type = type ? type->base_type : GLSL_TYPE_ERROR; + this->reladdr = NULL; + this->reladdr2 = NULL; + this->has_index2 = false; + this->double_reg2 = false; + this->array_id = array_id; + this->is_double_vertex_input = false; +} + +st_src_reg::st_src_reg(gl_register_file file, int index, enum glsl_base_type type) +{ + assert(file != PROGRAM_ARRAY); /* need array_id > 0 */ + this->type = type; + this->file = file; + this->index = index; + this->index2D = 0; + this->swizzle = SWIZZLE_XYZW; + this->negate = 0; + this->abs = 0; + this->reladdr = NULL; + this->reladdr2 = NULL; + this->has_index2 = false; + this->double_reg2 = false; + this->array_id = 0; + this->is_double_vertex_input = false; +} + +st_src_reg::st_src_reg(gl_register_file file, int index, enum glsl_base_type type, int index2D) +{ + assert(file != PROGRAM_ARRAY); /* need array_id > 0 */ + this->type = type; + this->file = file; + this->index = index; + this->index2D = index2D; + this->swizzle = SWIZZLE_XYZW; + this->negate = 0; + this->abs = 0; + this->reladdr = NULL; + this->
[Mesa-dev] [PATCH v7 6/6] mesa/st: glsl_to_tgsi: tie in new temporary register merge approach
This patch replaces the old register lifetime estiamtion and rename mapping evaluation with the new one. Performance to compare between the current and the new implementation were measured by running the shader-db in one thread. --- old new(std::sort) time ./run -j shaders real 5.80s 5.75s user 5.75s 5.70s sys 0.05s 0.05s valgrind --tool=callgrind --dump-instr=yes merge 0.08% 0.18% estimate lifetime 0.02% 0.11% evaluate mapping (incl=0.3%) 0.04% apply mapping 0.03% 0.02% --- perf (approximate because of statistic sampling) merge (total)0.09% 0.16% estimate lifetime0.03% 0.10% evaluate mapping (incl=0.02%) 0.04% apply mapping0.04% 0.04% --- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 58 ++ 1 file changed, 11 insertions(+), 47 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index a9c2584f26..3f311d6668 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -55,7 +55,7 @@ #include "st_glsl_types.h" #include "st_nir.h" #include "st_shader_cache.h" -#include "st_glsl_to_tgsi_private.h" +#include "st_glsl_to_tgsi_temprename.h" #include "util/hash_table.h" #include @@ -5145,54 +5145,18 @@ glsl_to_tgsi_visitor::merge_two_dsts(void) void glsl_to_tgsi_visitor::merge_registers(void) { - int *last_reads = ralloc_array(mem_ctx, int, this->next_temp); - int *first_writes = ralloc_array(mem_ctx, int, this->next_temp); - struct rename_reg_pair *renames = rzalloc_array(mem_ctx, struct rename_reg_pair, this->next_temp); - int i, j; - - /* Read the indices of the last read and first write to each temp register -* into an array so that we don't have to traverse the instruction list as -* much. */ - for (i = 0; i < this->next_temp; i++) { - last_reads[i] = -1; - first_writes[i] = -1; - } - get_last_temp_read_first_temp_write(last_reads, first_writes); - - /* Start looking for registers with non-overlapping usages that can be -* merged together. */ - for (i = 0; i < this->next_temp; i++) { - /* Don't touch unused registers. */ - if (last_reads[i] < 0 || first_writes[i] < 0) continue; - - for (j = 0; j < this->next_temp; j++) { - /* Don't touch unused registers. */ - if (last_reads[j] < 0 || first_writes[j] < 0) continue; - - /* We can merge the two registers if the first write to j is after or - * in the same instruction as the last read from i. Note that the - * register at index i will always be used earlier or at the same time - * as the register at index j. */ - if (first_writes[i] <= first_writes[j] && - last_reads[i] <= first_writes[j]) { -renames[j].new_reg = i; -renames[j].valid = true; - -/* Update the first_writes and last_reads arrays with the new - * values for the merged register index, and mark the newly unused - * register index as such. */ -assert(last_reads[j] >= last_reads[i]); -last_reads[i] = last_reads[j]; -first_writes[j] = -1; -last_reads[j] = -1; - } - } - } - + struct rename_reg_pair *renames = + rzalloc_array(mem_ctx, struct rename_reg_pair, this->next_temp); + struct lifetime *lifetimes = + rzalloc_array(mem_ctx, struct lifetime, this->next_temp); + + get_temp_registers_required_lifetimes(mem_ctx, &this->instructions, + this->next_temp, lifetimes); + get_temp_registers_remapping(mem_ctx, this->next_temp, lifetimes, renames); rename_temp_registers(renames); + + ralloc_free(lifetimes); ralloc_free(renames); - ralloc_free(last_reads); - ralloc_free(first_writes); } /* Reassign indices to temporary registers by reusing unused indices created -- 2.13.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v7 4/6] mesa/st: glsl_to_tgsi: add register rename mapping evaluator
The remapping evaluator first sorts the temporary registers ascending based on their first life time instruction, and then uses a binary search to find merge canidates. For the initial sorting it uses std::sort because qsort is quite slow in comparison. By removing the define USE_STL_SORT in src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp one can enable the alternative code path that uses qsort. Registers that are not written to are not considered for renaming since in glsl_to_tgsi_visitor::renumber_registers they are eliminated anyway. --- .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 117 + .../state_tracker/st_glsl_to_tgsi_temprename.h | 12 +++ 2 files changed, 129 insertions(+) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp index 46a707ec74..96427f75b2 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp @@ -590,6 +590,19 @@ lifetime temp_comp_access::get_required_lifetime() return make_lifetime(first_write, last_read); } +/* Helper class for sorting and searching the registers based + * on life times. */ +struct access_record { + int begin; + int end; + int reg; + bool erase; + + bool operator < (const access_record& rhs) const { + return begin < rhs.begin; + } +}; + } #ifndef NDEBUG @@ -784,6 +797,110 @@ get_temp_registers_required_lifetimes(void *mem_ctx, exec_list *instructions, delete[] acc; } +/* Find the next register between [start, end) that has a life time starting + * at or after bound by using a binary search. + * start points at the beginning of the search range, + * end points at the element past the end of the search range, and + * the array comprising [start, end) must be sorted in ascending order. + */ +static access_record* +find_next_rename(access_record* start, access_record* end, int bound) +{ + int delta = (end - start); + + while (delta > 0) { + int half = delta >> 1; + access_record* middle = start + half; + + if (bound <= middle->begin) { + delta = half; + } else { + start = middle; + ++start; + delta -= half + 1; + } + } + + return start; +} + +#ifndef USE_STL_SORT +static int access_record_compare (const void *a, const void *b) { + const access_record *aa = static_cast(a); + const access_record *bb = static_cast(b); + return aa->begin < bb->begin ? -1 : (aa->begin > bb->begin ? 1 : 0); +} +#endif + +/* This functions evaluates the register merges by using an buínary + * search to find suitable merge candidates. */ +void get_temp_registers_remapping(void *mem_ctx, int ntemps, + const struct lifetime* lifetimes, + struct rename_reg_pair *result) +{ + access_record *reg_access = ralloc_array(mem_ctx, access_record, ntemps); + + int used_temps = 0; + for (int i = 0; i < ntemps; ++i) { + if (lifetimes[i].begin >= 0) { + reg_access[used_temps].begin = lifetimes[i].begin; + reg_access[used_temps].end = lifetimes[i].end; + reg_access[used_temps].reg = i; + reg_access[used_temps].erase = false; + ++used_temps; + } + } + +#ifdef USE_STL_SORT + std::sort(reg_access, reg_access + used_temps); +#else + std::qsort(reg_access, used_temps, sizeof(access_record), access_record_compare); +#endif + + access_record *trgt = reg_access; + access_record *reg_access_end = reg_access + used_temps; + access_record *first_erase = reg_access_end; + access_record *search_start = trgt + 1; + + while (trgt != reg_access_end) { + access_record *src = find_next_rename(search_start, reg_access_end, +trgt->end); + if (src != reg_access_end) { + result[src->reg].new_reg = trgt->reg; + result[src->reg].valid = true; + trgt->end = src->end; + + /* Since we only search forward, don't remove the renamed + * register just now, only mark it. */ + src->erase = true; + + if (first_erase == reg_access_end) +first_erase = src; + + search_start = src + 1; + } else { + /* Moving to the next target register it is time to remove + * the already merged registers from the search range */ + if (first_erase != reg_access_end) { +access_record *outp = first_erase; +access_record *inp = first_erase + 1; + +while (inp != reg_access_end) { + if (!inp->erase) + *outp++ = *inp; + ++inp; +} + +reg_access_end = outp; +first_erase = reg_access_end; + } + ++trgt; + search_start = trgt + 1; + } + } + ralloc_free(reg_access); +} + /* Code below used for debugging */ #ifndef NDEBUG static const char s
[Mesa-dev] [PATCH v7 3/6] mesa/st: glsl_to_tgsi: add tests for the new temporary lifetime tracker
This patch adds a set of unit tests for the new lifetime tracker. --- configure.ac |1 + src/mesa/Makefile.am |2 +- src/mesa/state_tracker/tests/Makefile.am | 36 + .../tests/test_glsl_to_tgsi_lifetime.cpp | 1405 4 files changed, 1443 insertions(+), 1 deletion(-) create mode 100644 src/mesa/state_tracker/tests/Makefile.am create mode 100644 src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp diff --git a/configure.ac b/configure.ac index 46fcd8f3fe..0a299ea21a 100644 --- a/configure.ac +++ b/configure.ac @@ -2851,6 +2851,7 @@ AC_CONFIG_FILES([Makefile src/mesa/drivers/osmesa/osmesa.pc src/mesa/drivers/x11/Makefile src/mesa/main/tests/Makefile + src/mesa/state_tracker/tests/Makefile src/util/Makefile src/util/tests/hash_table/Makefile src/vulkan/Makefile]) diff --git a/src/mesa/Makefile.am b/src/mesa/Makefile.am index 97a9bbd8c2..865735be27 100644 --- a/src/mesa/Makefile.am +++ b/src/mesa/Makefile.am @@ -19,7 +19,7 @@ # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS # IN THE SOFTWARE. -SUBDIRS = . main/tests +SUBDIRS = . main/tests state_tracker/tests if HAVE_XLIB_GLX SUBDIRS += drivers/x11 diff --git a/src/mesa/state_tracker/tests/Makefile.am b/src/mesa/state_tracker/tests/Makefile.am new file mode 100644 index 00..fb64cf9dc2 --- /dev/null +++ b/src/mesa/state_tracker/tests/Makefile.am @@ -0,0 +1,36 @@ +AM_CFLAGS = \ + $(PTHREAD_CFLAGS) + +AM_CXXFLAGS = \ + $(LLVM_CXXFLAGS) + +AM_CPPFLAGS = \ + -I$(top_srcdir)/src/gtest/include \ + -I$(top_srcdir)/src \ + -I$(top_srcdir)/src/mapi \ + -I$(top_builddir)/src/mesa \ + -I$(top_srcdir)/src/mesa \ + -I$(top_srcdir)/include \ + -I$(top_srcdir)/src/gallium/include \ + -I$(top_srcdir)/src/gallium/auxiliary \ + $(DEFINES) + +TESTS = st-renumerate-test +check_PROGRAMS = st-renumerate-test + +st_renumerate_test_SOURCES = \ + test_glsl_to_tgsi_lifetime.cpp + +st_renumerate_test_LDFLAGS = \ + $(LLVM_LDFLAGS) + +st_renumerate_test_LDADD = \ + $(top_builddir)/src/mesa/libmesagallium.la \ + $(top_builddir)/src/mapi/shared-glapi/libglapi.la \ + $(top_builddir)/src/gallium/auxiliary/libgallium.la \ + $(top_builddir)/src/util/libmesautil.la \ + $(top_builddir)/src/gtest/libgtest.la \ + $(GALLIUM_COMMON_LIB_DEPS) \ + $(LLVM_LIBS) \ + $(PTHREAD_LIBS) \ + $(DLOPEN_LIBS) diff --git a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp new file mode 100644 index 00..b9dadcc03d --- /dev/null +++ b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp @@ -0,0 +1,1405 @@ +/* + * Copyright © 2017 Gert Wollny + * + * 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 (including the next + * paragraph) 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 +#include +#include +#include +#include + +#include +#include + +using std::vector; +using std::pair; +using std::make_pair; + +/* A line to describe a TGSI instruction for building mock shaders. */ +struct MockCodeline { + MockCodeline(unsigned _op): op(_op) {} + MockCodeline(unsigned _op, const vector& _dst, const vector& _src, const vector&_to): + op(_op), dst(_dst), src(_src), tex_offsets(_to){} + unsigned op; + vector dst; + vector src; + vector tex_offsets; +}; + +/* A line to describe a TGSI instruction with swizzeling and write makss + * for building mock shaders. + */ +struct MockCodelineWithSwizzle { + MockCodelineWithSwizzle(unsigned _op): op(_op) {} + MockCodelineWithSwizzle(unsigned _op, const vector>& _dst, + const vector>& _src,
Re: [Mesa-dev] [PATCH v7 0/6] mesa/st: glsl_to_tgsi: improved temp-reg lifetime estimation
Hi, the patch no longer applies to mesa master. I've pushed the rebased patch set to my github mirror (see below), but should I also send the corrected patches here? Best, Gert Am Montag, den 17.07.2017, 17:47 +0200 schrieb Gert Wollny: > Hello Nicolai, > > many thanks for the review. With this new patch set I've implemented > most of your > suggestions, patch 1 and 6 are only rebased though. > The patches are also available via > > https://github.com/gerddie/mesa/tree/regrename-v7 > > The one thing I did not do (or better said, I reverted it) is using > _mesa_register_file_name in the debug output because the names > returned are not > correct for the TGSI file indices I have access to there. > > Piglit doesn't show any regressions and the changes are: > > - Rebase to 9ee67467 > - Correct documentation of GLSL_TO_TGSI_RENAME_DEBUG in commit > message. > - Fix typos, include files, formatting, and some variable names. > - Add anonymous namespace around classes. > - Replace debug_log singleton by a function. > - Cleanup the switch-case scope creation. > - Track switch variable only for SWITCH statement. > - Add test case with switch in loop that has a case where the > register is > written, then falls through where it is read and correct code > accordingly. > Specifically, this showed that you were right that the scope of the > case that falls through must not be extended. > - Add test case for more then one break in loop and make sure the > first break > is used for life-time tracking. > - Simplify access flag tracking and merging. > - Drop special handling of registers that are only read, i.e. treat > them like > unused registers since renumber_registers will do this anyway. > - Simplify the enclosing scope resolution. > - Handle TEMP[0] just like the others. Somehow I had the idea that > TEMP[0] is > special and should not be touched, and the debug output running > various > programs and also from piglit shaders doesn't show TEMP[0] as > used. > - Optimize renaming evaluation by only copying used registers to the > reg_access array that is used for the evaluation. > > Best, > Gert > > Gert Wollny (6): > mesa/st: glsl_to_tgsi move some helper classes to extra files > mesa/st: glsl_to_tgsi: implement new temporary register lifetime > tracker > mesa/st: glsl_to_tgsi: add tests for the new temporary lifetime > tracker > mesa/st: glsl_to_tgsi: add register rename mapping evaluator > mesa/st: glsl_to_tgsi: Add test set for evaluation of rename > mapping > mesa/st: glsl_to_tgsi: tie in new temporary register merge approach > > configure.ac |1 + > src/mesa/Makefile.am |2 +- > src/mesa/Makefile.sources |4 + > src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 344 + > src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp | 196 +++ > src/mesa/state_tracker/st_glsl_to_tgsi_private.h | 167 ++ > .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 992 > > .../state_tracker/st_glsl_to_tgsi_temprename.h | 67 + > src/mesa/state_tracker/tests/Makefile.am | 36 + > .../tests/test_glsl_to_tgsi_lifetime.cpp | 1597 > > 10 files changed, 3073 insertions(+), 333 deletions(-) > create mode 100644 > src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp > create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_private.h > create mode 100644 > src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp > create mode 100644 > src/mesa/state_tracker/st_glsl_to_tgsi_temprename.h > create mode 100644 src/mesa/state_tracker/tests/Makefile.am > create mode 100644 > src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] Revert "st_glsl_to_tgsi: rewrite rename registers to use array fully."
Am Dienstag, den 01.08.2017, 09:32 +1000 schrieb Dave Airlie: > From: Dave Airlie > > This reverts commit 3008161d28e38336ba39aba4769a2deaf9732f55, > which caused a regression for VMWare. > > The initial code had some recursion in it, that I removed by accident > trying to add back the recursion broke lots of things, take the high > road and revert for now. Since I've prepared a patch set that improves the register merging [1] that is awaiting another review I'm wondering which part broke the recursion? [1] uses Davids rename_temp_registers method, but changes the initial life-time estimation, and should improve the all-over situation for drivers with limited registers (it does for R600g). It would be nice to see if it also helps with the VMWare driver and avoids the regression. (The latest version of the patch set does not apply to master though, an rebased version can be found at [2]). many thanks, Gert [1] https://patchwork.freedesktop.org/series/25594/ [2] https://github.com/gerddie/mesa/tree/regrename-v7 > > Fixes: 3008161d (st_glsl_to_tgsi: rewrite rename registers to use > array fully.) > Reviewed-by: Brian Paul > Tested-by: Brian Paul > Signed-off-by: Dave Airlie > --- > src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 55 -- > > 1 file changed, 29 insertions(+), 26 deletions(-) > > diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > index 3983fe7..d496fff 100644 > --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp > @@ -399,7 +399,7 @@ find_array_type(struct inout_decl *decls, > unsigned count, unsigned array_id) > } > > struct rename_reg_pair { > - bool valid; > + int old_reg; > int new_reg; > }; > > @@ -568,7 +568,7 @@ public: > > void simplify_cmp(void); > > - void rename_temp_registers(struct rename_reg_pair *renames); > + void rename_temp_registers(int num_renames, struct > rename_reg_pair *renames); > void get_first_temp_read(int *first_reads); > void get_first_temp_write(int *first_writes); > void get_last_temp_read_first_temp_write(int *last_reads, int > *first_writes); > @@ -4835,37 +4835,36 @@ glsl_to_tgsi_visitor::simplify_cmp(void) > > /* Replaces all references to a temporary register index with > another index. */ > void > -glsl_to_tgsi_visitor::rename_temp_registers(struct rename_reg_pair > *renames) > +glsl_to_tgsi_visitor::rename_temp_registers(int num_renames, struct > rename_reg_pair *renames) > { > foreach_in_list(glsl_to_tgsi_instruction, inst, &this- > >instructions) { > unsigned j; > + int k; > for (j = 0; j < num_inst_src_regs(inst); j++) { > - if (inst->src[j].file == PROGRAM_TEMPORARY) { > -int old_idx = inst->src[j].index; > -if (renames[old_idx].valid) > - inst->src[j].index = renames[old_idx].new_reg; > - } > + if (inst->src[j].file == PROGRAM_TEMPORARY) > +for (k = 0; k < num_renames; k++) > + if (inst->src[j].index == renames[k].old_reg) > + inst->src[j].index = renames[k].new_reg; > } > > for (j = 0; j < inst->tex_offset_num_offset; j++) { > - if (inst->tex_offsets[j].file == PROGRAM_TEMPORARY) { > -int old_idx = inst->tex_offsets[j].index; > -if (renames[old_idx].valid) > - inst->tex_offsets[j].index = > renames[old_idx].new_reg; > - } > + if (inst->tex_offsets[j].file == PROGRAM_TEMPORARY) > +for (k = 0; k < num_renames; k++) > + if (inst->tex_offsets[j].index == renames[k].old_reg) > + inst->tex_offsets[j].index = renames[k].new_reg; > } > > if (inst->resource.file == PROGRAM_TEMPORARY) { > - int old_idx = inst->resource.index; > - if (renames[old_idx].valid) > -inst->resource.index = renames[old_idx].new_reg; > + for (k = 0; k < num_renames; k++) > +if (inst->resource.index == renames[k].old_reg) > + inst->resource.index = renames[k].new_reg; > } > > for (j = 0; j < num_inst_dst_regs(inst); j++) { > - if (inst->dst[j].file == PROGRAM_TEMPORARY) { > -int old_idx = inst->dst[j].index; > -if (renames[old_idx].valid) > - inst->dst[j].index = renames[old_idx].new_reg;} > + if (inst->dst[j].file == PROGRAM_TEMPORARY) > + for (k = 0; k < num_renames; k++) > +if (inst->dst[j].index == renames[k].old_reg) > + inst->dst[j].index = renames[k].new_reg; > } > } > } > @@ -5446,6 +5445,7 @@ glsl_to_tgsi_visitor::merge_registers(void) > int *first_writes = ralloc_array(mem_ctx, int, this->next_temp); > struct rename_reg_pair *renames = rzalloc_array(mem_ctx, struct > rename_reg_pair, this->next_temp); > int i, j; > + int
Re: [Mesa-dev] [PATCH] Revert "st_glsl_to_tgsi: rewrite rename registers to use array fully."
> The original code has a loop over num_renames, so if two renames > were in the list in order like > > 8 renames to 4 and 4 renames to 2, then 8 would rename to 2. > however if 4 renamed to 2 then 8 renamed to 4 (not sure if this could > happen), it was different. > > My initial patch fails because it only does one transition per slot, > so if 8 renames to 4, then nothing renames it later to 2. I see, well, with my patch will not fail for this reason, since a valid rename will always assign the final register index when evaluating the mapping (see 4/6), so if it will accepted than it can (and should) be merged with your rename_temp_registers method. best, Gert ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v8 0/7] mesa/st: glsl_to_tgsi: improved temp-reg lifetime estimation
Dear all, This is mostly a resend of version 7 of the patch rebased to 1e696b962b72. The only other notable change is that I added Dave Airlie's patch for register renaming (slightly changed to account for the inst->resource_index). My code makes use of his renaming methods, and since my mapping evaluator doesn't make use of a recursive renaming strategy, the problem that made Dave revert this patch does not occure after applying the series. If prefered I could also merge that patch with my series, but personally I prefer to keep it seperate. Many thanks for any reviews, Gert Dave Airlie (1): st_glsl_to_tgsi: rewrite rename registers to use array fully. Gert Wollny (6): mesa/st: glsl_to_tgsi move some helper classes to extra files mesa/st: glsl_to_tgsi: implement new temporary register lifetime tracker mesa/st: glsl_to_tgsi: add tests for the new temporary lifetime tracker mesa/st: glsl_to_tgsi: add register rename mapping evaluator mesa/st: glsl_to_tgsi: Add test set for evaluation of rename mapping mesa/st: glsl_to_tgsi: tie in new temporary register merge approach configure.ac |1 + src/mesa/Makefile.am |2 +- src/mesa/Makefile.sources |4 + src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 390 + src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp | 196 +++ src/mesa/state_tracker/st_glsl_to_tgsi_private.h | 168 ++ .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 998 .../state_tracker/st_glsl_to_tgsi_temprename.h | 67 + src/mesa/state_tracker/tests/Makefile.am | 36 + .../tests/test_glsl_to_tgsi_lifetime.cpp | 1597 10 files changed, 3101 insertions(+), 358 deletions(-) create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_private.h create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_temprename.h create mode 100644 src/mesa/state_tracker/tests/Makefile.am create mode 100644 src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp -- 2.13.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v8 7/7] mesa/st: glsl_to_tgsi: tie in new temporary register merge approach
This patch replaces the old register lifetime estiamtion and rename mapping evaluation with the new one. Performance to compare between the current and the new implementation were measured by running the shader-db in one thread. --- old new(std::sort) time ./run -j1 shaders real 5.80s 5.75s user 5.75s 5.70s sys 0.05s 0.05s valgrind --tool=callgrind --dump-instr=yes merge 0.08% 0.18% estimate lifetime 0.02% 0.11% evaluate mapping (incl=0.3%) 0.04% apply mapping 0.03% 0.02% --- perf (approximate because of statistic sampling) merge (total)0.09% 0.16% estimate lifetime0.03% 0.10% evaluate mapping (incl=0.02%) 0.04% apply mapping0.04% 0.04% --- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 58 ++ 1 file changed, 11 insertions(+), 47 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index 62d1a7936f..18ba8819f4 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -54,7 +54,7 @@ #include "st_format.h" #include "st_nir.h" #include "st_shader_cache.h" -#include "st_glsl_to_tgsi_private.h" +#include "st_glsl_to_tgsi_temprename.h" #include "util/hash_table.h" #include @@ -5157,54 +5157,18 @@ glsl_to_tgsi_visitor::merge_two_dsts(void) void glsl_to_tgsi_visitor::merge_registers(void) { - int *last_reads = ralloc_array(mem_ctx, int, this->next_temp); - int *first_writes = ralloc_array(mem_ctx, int, this->next_temp); - struct rename_reg_pair *renames = rzalloc_array(mem_ctx, struct rename_reg_pair, this->next_temp); - int i, j; - - /* Read the indices of the last read and first write to each temp register -* into an array so that we don't have to traverse the instruction list as -* much. */ - for (i = 0; i < this->next_temp; i++) { - last_reads[i] = -1; - first_writes[i] = -1; - } - get_last_temp_read_first_temp_write(last_reads, first_writes); - - /* Start looking for registers with non-overlapping usages that can be -* merged together. */ - for (i = 0; i < this->next_temp; i++) { - /* Don't touch unused registers. */ - if (last_reads[i] < 0 || first_writes[i] < 0) continue; - - for (j = 0; j < this->next_temp; j++) { - /* Don't touch unused registers. */ - if (last_reads[j] < 0 || first_writes[j] < 0) continue; - - /* We can merge the two registers if the first write to j is after or - * in the same instruction as the last read from i. Note that the - * register at index i will always be used earlier or at the same time - * as the register at index j. */ - if (first_writes[i] <= first_writes[j] && - last_reads[i] <= first_writes[j]) { -renames[j].new_reg = i; -renames[j].valid = true; - -/* Update the first_writes and last_reads arrays with the new - * values for the merged register index, and mark the newly unused - * register index as such. */ -assert(last_reads[j] >= last_reads[i]); -last_reads[i] = last_reads[j]; -first_writes[j] = -1; -last_reads[j] = -1; - } - } - } - + struct rename_reg_pair *renames = + rzalloc_array(mem_ctx, struct rename_reg_pair, this->next_temp); + struct lifetime *lifetimes = + rzalloc_array(mem_ctx, struct lifetime, this->next_temp); + + get_temp_registers_required_lifetimes(mem_ctx, &this->instructions, + this->next_temp, lifetimes); + get_temp_registers_remapping(mem_ctx, this->next_temp, lifetimes, renames); rename_temp_registers(renames); + + ralloc_free(lifetimes); ralloc_free(renames); - ralloc_free(last_reads); - ralloc_free(first_writes); } /* Reassign indices to temporary registers by reusing unused indices created -- 2.13.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v8 4/7] mesa/st: glsl_to_tgsi: add tests for the new temporary lifetime tracker
This patch adds a set of unit tests for the new lifetime tracker. --- configure.ac |1 + src/mesa/Makefile.am |2 +- src/mesa/state_tracker/tests/Makefile.am | 36 + .../tests/test_glsl_to_tgsi_lifetime.cpp | 1405 4 files changed, 1443 insertions(+), 1 deletion(-) create mode 100644 src/mesa/state_tracker/tests/Makefile.am create mode 100644 src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp diff --git a/configure.ac b/configure.ac index 5b12dd8506..05fa6e369a 100644 --- a/configure.ac +++ b/configure.ac @@ -2903,6 +2903,7 @@ AC_CONFIG_FILES([Makefile src/mesa/drivers/osmesa/osmesa.pc src/mesa/drivers/x11/Makefile src/mesa/main/tests/Makefile + src/mesa/state_tracker/tests/Makefile src/util/Makefile src/util/tests/hash_table/Makefile src/util/xmlpool/Makefile diff --git a/src/mesa/Makefile.am b/src/mesa/Makefile.am index 97a9bbd8c2..865735be27 100644 --- a/src/mesa/Makefile.am +++ b/src/mesa/Makefile.am @@ -19,7 +19,7 @@ # FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS # IN THE SOFTWARE. -SUBDIRS = . main/tests +SUBDIRS = . main/tests state_tracker/tests if HAVE_XLIB_GLX SUBDIRS += drivers/x11 diff --git a/src/mesa/state_tracker/tests/Makefile.am b/src/mesa/state_tracker/tests/Makefile.am new file mode 100644 index 00..fb64cf9dc2 --- /dev/null +++ b/src/mesa/state_tracker/tests/Makefile.am @@ -0,0 +1,36 @@ +AM_CFLAGS = \ + $(PTHREAD_CFLAGS) + +AM_CXXFLAGS = \ + $(LLVM_CXXFLAGS) + +AM_CPPFLAGS = \ + -I$(top_srcdir)/src/gtest/include \ + -I$(top_srcdir)/src \ + -I$(top_srcdir)/src/mapi \ + -I$(top_builddir)/src/mesa \ + -I$(top_srcdir)/src/mesa \ + -I$(top_srcdir)/include \ + -I$(top_srcdir)/src/gallium/include \ + -I$(top_srcdir)/src/gallium/auxiliary \ + $(DEFINES) + +TESTS = st-renumerate-test +check_PROGRAMS = st-renumerate-test + +st_renumerate_test_SOURCES = \ + test_glsl_to_tgsi_lifetime.cpp + +st_renumerate_test_LDFLAGS = \ + $(LLVM_LDFLAGS) + +st_renumerate_test_LDADD = \ + $(top_builddir)/src/mesa/libmesagallium.la \ + $(top_builddir)/src/mapi/shared-glapi/libglapi.la \ + $(top_builddir)/src/gallium/auxiliary/libgallium.la \ + $(top_builddir)/src/util/libmesautil.la \ + $(top_builddir)/src/gtest/libgtest.la \ + $(GALLIUM_COMMON_LIB_DEPS) \ + $(LLVM_LIBS) \ + $(PTHREAD_LIBS) \ + $(DLOPEN_LIBS) diff --git a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp new file mode 100644 index 00..b9dadcc03d --- /dev/null +++ b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp @@ -0,0 +1,1405 @@ +/* + * Copyright © 2017 Gert Wollny + * + * 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 (including the next + * paragraph) 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 +#include +#include +#include +#include + +#include +#include + +using std::vector; +using std::pair; +using std::make_pair; + +/* A line to describe a TGSI instruction for building mock shaders. */ +struct MockCodeline { + MockCodeline(unsigned _op): op(_op) {} + MockCodeline(unsigned _op, const vector& _dst, const vector& _src, const vector&_to): + op(_op), dst(_dst), src(_src), tex_offsets(_to){} + unsigned op; + vector dst; + vector src; + vector tex_offsets; +}; + +/* A line to describe a TGSI instruction with swizzeling and write makss + * for building mock shaders. + */ +struct MockCodelineWithSwizzle { + MockCodelineWithSwizzle(unsigned _op): op(_op) {} + MockCodelineWithSwizzle(unsigned _op, const vector>& _dst, +
[Mesa-dev] [PATCH v8 5/7] mesa/st: glsl_to_tgsi: add register rename mapping evaluator
The remapping evaluator first sorts the temporary registers ascending based on their first life time instruction, and then uses a binary search to find merge canidates. For the initial sorting it uses std::sort because qsort is quite slow in comparison. By removing the define USE_STL_SORT in src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp one can enable the alternative code path that uses qsort. Registers that are not written to are not considered for renaming since in glsl_to_tgsi_visitor::renumber_registers they are eliminated anyway. --- .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 117 + .../state_tracker/st_glsl_to_tgsi_temprename.h | 12 +++ 2 files changed, 129 insertions(+) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp index 46a707ec74..96427f75b2 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp @@ -590,6 +590,19 @@ lifetime temp_comp_access::get_required_lifetime() return make_lifetime(first_write, last_read); } +/* Helper class for sorting and searching the registers based + * on life times. */ +struct access_record { + int begin; + int end; + int reg; + bool erase; + + bool operator < (const access_record& rhs) const { + return begin < rhs.begin; + } +}; + } #ifndef NDEBUG @@ -784,6 +797,110 @@ get_temp_registers_required_lifetimes(void *mem_ctx, exec_list *instructions, delete[] acc; } +/* Find the next register between [start, end) that has a life time starting + * at or after bound by using a binary search. + * start points at the beginning of the search range, + * end points at the element past the end of the search range, and + * the array comprising [start, end) must be sorted in ascending order. + */ +static access_record* +find_next_rename(access_record* start, access_record* end, int bound) +{ + int delta = (end - start); + + while (delta > 0) { + int half = delta >> 1; + access_record* middle = start + half; + + if (bound <= middle->begin) { + delta = half; + } else { + start = middle; + ++start; + delta -= half + 1; + } + } + + return start; +} + +#ifndef USE_STL_SORT +static int access_record_compare (const void *a, const void *b) { + const access_record *aa = static_cast(a); + const access_record *bb = static_cast(b); + return aa->begin < bb->begin ? -1 : (aa->begin > bb->begin ? 1 : 0); +} +#endif + +/* This functions evaluates the register merges by using an buínary + * search to find suitable merge candidates. */ +void get_temp_registers_remapping(void *mem_ctx, int ntemps, + const struct lifetime* lifetimes, + struct rename_reg_pair *result) +{ + access_record *reg_access = ralloc_array(mem_ctx, access_record, ntemps); + + int used_temps = 0; + for (int i = 0; i < ntemps; ++i) { + if (lifetimes[i].begin >= 0) { + reg_access[used_temps].begin = lifetimes[i].begin; + reg_access[used_temps].end = lifetimes[i].end; + reg_access[used_temps].reg = i; + reg_access[used_temps].erase = false; + ++used_temps; + } + } + +#ifdef USE_STL_SORT + std::sort(reg_access, reg_access + used_temps); +#else + std::qsort(reg_access, used_temps, sizeof(access_record), access_record_compare); +#endif + + access_record *trgt = reg_access; + access_record *reg_access_end = reg_access + used_temps; + access_record *first_erase = reg_access_end; + access_record *search_start = trgt + 1; + + while (trgt != reg_access_end) { + access_record *src = find_next_rename(search_start, reg_access_end, +trgt->end); + if (src != reg_access_end) { + result[src->reg].new_reg = trgt->reg; + result[src->reg].valid = true; + trgt->end = src->end; + + /* Since we only search forward, don't remove the renamed + * register just now, only mark it. */ + src->erase = true; + + if (first_erase == reg_access_end) +first_erase = src; + + search_start = src + 1; + } else { + /* Moving to the next target register it is time to remove + * the already merged registers from the search range */ + if (first_erase != reg_access_end) { +access_record *outp = first_erase; +access_record *inp = first_erase + 1; + +while (inp != reg_access_end) { + if (!inp->erase) + *outp++ = *inp; + ++inp; +} + +reg_access_end = outp; +first_erase = reg_access_end; + } + ++trgt; + search_start = trgt + 1; + } + } + ralloc_free(reg_access); +} + /* Code below used for debugging */ #ifndef NDEBUG static const char s
[Mesa-dev] [PATCH v8 6/7] mesa/st: glsl_to_tgsi: Add test set for evaluation of rename mapping
The patch adds tests for the register rename mapping evaluation and combined life time estimation and renaming. --- .../tests/test_glsl_to_tgsi_lifetime.cpp | 192 + 1 file changed, 192 insertions(+) diff --git a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp index b9dadcc03d..f4820d4777 100644 --- a/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp +++ b/src/mesa/state_tracker/tests/test_glsl_to_tgsi_lifetime.cpp @@ -124,6 +124,33 @@ protected: void check(const vector& result, const expectation& e); }; +/* With this test class the renaming mapping estimation is tested */ +class RegisterRemappingTest : public MesaTestWithMemCtx { +protected: + void run(const vector& lt, const vector& expect); +}; + +/* With this test class the combined lifetime estimation and renaming + * mepping estimation is tested + */ +class RegisterLifetimeAndRemappingTest : public RegisterRemappingTest { +protected: + using RegisterRemappingTest::run; + template + void run(const vector& code, const vector& expect); +}; + +template +void RegisterLifetimeAndRemappingTest::run(const vector& code, + const vector& expect) +{ + MockShader shader(code); + std::vector lt(shader.get_num_temps()); + get_temp_registers_required_lifetimes(mem_ctx, shader.get_program(), + shader.get_num_temps(), <[0]); + this->run(lt, expect); +} + TEST_F(LifetimeEvaluatorExactTest, SimpleMoveAdd) { const vector code = { @@ -1175,6 +1202,148 @@ TEST_F(LifetimeEvaluatorExactTest, NestedLoopWithWriteAfterBreak) run (code, expectation({{-1,-1}, {0,8}})); } +/* Test remapping table of registers. The tests don't assume + * that the sorting algorithm used to sort the lifetimes + * based on their 'begin' is stable. + */ +TEST_F(RegisterRemappingTest, RegisterRemapping1) +{ + vector lt({{-1,-1}, +{0,1}, +{0,2}, +{1,2}, +{2,10}, +{3,5}, +{5,10} + }); + + vector expect({0,1,2,1,1,2,2}); + run(lt, expect); +} + +TEST_F(RegisterRemappingTest, RegisterRemapping2) +{ + vector lt({{-1,-1}, +{0,1}, +{0,2}, +{3,4}, +{4,5}, + }); + vector expect({0,1,2,1,1}); + run(lt, expect); +} + +TEST_F(RegisterRemappingTest, RegisterRemappingMergeAllToOne) +{ + vector lt({{-1,-1}, +{0,1}, +{1,2}, +{2,3}, +{3,4}, + }); + vector expect({0,1,1,1,1}); + run(lt, expect); +} + +TEST_F(RegisterRemappingTest, RegisterRemappingIgnoreUnused) +{ + vector lt({{-1,-1}, +{0,1}, +{1,2}, +{2,3}, +{-1,-1}, +{3,4}, + }); + vector expect({0,1,1,1,4,1}); + run(lt, expect); +} + +TEST_F(RegisterRemappingTest, RegisterRemappingMergeZeroLifetimeRegisters) +{ + vector lt({{-1,-1}, +{0,1}, +{1,2}, +{2,3}, +{3,3}, +{3,4}, + }); + vector expect({0,1,1,1,1,1}); + run(lt, expect); +} + +TEST_F(RegisterLifetimeAndRemappingTest, LifetimeAndRemapping) +{ + const vector code = { + {TGSI_OPCODE_USEQ, {5}, {in0,in1}, {}}, + {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + {TGSI_OPCODE_UCMP, {1}, {5,in1,1}, {}}, + {TGSI_OPCODE_FSLT, {2}, {1,in1}, {}}, + {TGSI_OPCODE_UIF, {}, {2}, {}}, + { TGSI_OPCODE_MOV, {3}, {in1}, {}}, + {TGSI_OPCODE_ELSE}, + { TGSI_OPCODE_MOV, {4}, {in1}, {}}, + { TGSI_OPCODE_MOV, {4}, {4}, {}}, + { TGSI_OPCODE_MOV, {3}, {4}, {}}, + {TGSI_OPCODE_ENDIF}, + {TGSI_OPCODE_MOV, {out1}, {3}, {}}, + {TGSI_OPCODE_END} + }; + run (code, vector({0,1,5,5,1,5})); +} + +TEST_F(RegisterLifetimeAndRemappingTest, LifetimeAndRemappingWithUnusedReadOnlyIgnored) +{ + const vector code = { + {TGSI_OPCODE_USEQ, {1}, {in0,in1}, {}}, + {TGSI_OPCODE_UCMP, {2}, {1,in1,2}, {}}, + {TGSI_OPCODE_UCMP, {4}, {2,in1,1}, {}}, + {TGSI_OPCODE_ADD, {5}, {2,4}, {}}, + {TGSI_OPCODE_UIF, {}, {7}, {}}, + { TGSI_OPCODE_ADD, {8}, {5,4}, {}}, + {TGSI_OPCODE_ENDIF}, + {TGSI_OPCODE_MOV, {out1}, {8}, {}}, + {TGSI_OPCODE_END} + }; + /* lt: 1: 0-2,2: 1-3 3: u 4: 2-5 5: 3-5 6: u 7: 0-(-1),8: 5-7 */ + run (code, vector({0,1,2,3,1,2,6,7,1})); +} + +TEST_F(RegisterLifetimeAndRemappingTest, LifetimeAndRemap
[Mesa-dev] [PATCH v8 1/7] st_glsl_to_tgsi: rewrite rename registers to use array fully.
From: Dave Airlie Instead of having to search the whole array, just use the whole thing and store a valid bit in there with the rename. Removes this from the profile on some of the fp64 tests Reviewed-by: Timothy Arceri Signed-off-by: Dave Airlie --- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 55 ++ 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index 9bc745c791..a2497b2e0c 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -398,7 +398,7 @@ find_array_type(struct inout_decl *decls, unsigned count, unsigned array_id) } struct rename_reg_pair { - int old_reg; + bool valid; int new_reg; }; @@ -567,7 +567,7 @@ public: void simplify_cmp(void); - void rename_temp_registers(int num_renames, struct rename_reg_pair *renames); + void rename_temp_registers(struct rename_reg_pair *renames); void get_first_temp_read(int *first_reads); void get_first_temp_write(int *first_writes); void get_last_temp_read_first_temp_write(int *last_reads, int *first_writes); @@ -4835,36 +4835,37 @@ glsl_to_tgsi_visitor::simplify_cmp(void) /* Replaces all references to a temporary register index with another index. */ void -glsl_to_tgsi_visitor::rename_temp_registers(int num_renames, struct rename_reg_pair *renames) +glsl_to_tgsi_visitor::rename_temp_registers(struct rename_reg_pair *renames) { foreach_in_list(glsl_to_tgsi_instruction, inst, &this->instructions) { unsigned j; - int k; for (j = 0; j < num_inst_src_regs(inst); j++) { - if (inst->src[j].file == PROGRAM_TEMPORARY) -for (k = 0; k < num_renames; k++) - if (inst->src[j].index == renames[k].old_reg) - inst->src[j].index = renames[k].new_reg; + if (inst->src[j].file == PROGRAM_TEMPORARY) { +int old_idx = inst->src[j].index; +if (renames[old_idx].valid) + inst->src[j].index = renames[old_idx].new_reg; + } } for (j = 0; j < inst->tex_offset_num_offset; j++) { - if (inst->tex_offsets[j].file == PROGRAM_TEMPORARY) -for (k = 0; k < num_renames; k++) - if (inst->tex_offsets[j].index == renames[k].old_reg) - inst->tex_offsets[j].index = renames[k].new_reg; + if (inst->tex_offsets[j].file == PROGRAM_TEMPORARY) { +int old_idx = inst->tex_offsets[j].index; +if (renames[old_idx].valid) + inst->tex_offsets[j].index = renames[old_idx].new_reg; + } } if (inst->resource.file == PROGRAM_TEMPORARY) { - for (k = 0; k < num_renames; k++) -if (inst->resource.index == renames[k].old_reg) - inst->resource.index = renames[k].new_reg; + int old_idx = inst->resource.index; + if (renames[old_idx].valid) +inst->resource.index = renames[old_idx].new_reg; } for (j = 0; j < num_inst_dst_regs(inst); j++) { - if (inst->dst[j].file == PROGRAM_TEMPORARY) - for (k = 0; k < num_renames; k++) -if (inst->dst[j].index == renames[k].old_reg) - inst->dst[j].index = renames[k].new_reg; + if (inst->dst[j].file == PROGRAM_TEMPORARY) { +int old_idx = inst->dst[j].index; +if (renames[old_idx].valid) + inst->dst[j].index = renames[old_idx].new_reg;} } } } @@ -5445,7 +5446,6 @@ glsl_to_tgsi_visitor::merge_registers(void) int *first_writes = ralloc_array(mem_ctx, int, this->next_temp); struct rename_reg_pair *renames = rzalloc_array(mem_ctx, struct rename_reg_pair, this->next_temp); int i, j; - int num_renames = 0; /* Read the indices of the last read and first write to each temp register * into an array so that we don't have to traverse the instruction list as @@ -5472,9 +5472,8 @@ glsl_to_tgsi_visitor::merge_registers(void) * as the register at index j. */ if (first_writes[i] <= first_writes[j] && last_reads[i] <= first_writes[j]) { -renames[num_renames].old_reg = j; -renames[num_renames].new_reg = i; -num_renames++; +renames[j].new_reg = i; +renames[j].valid = true; /* Update the first_writes and last_reads arrays with the new * values for the merged register index, and mark the newly unused @@ -5487,7 +5486,7 @@ glsl_to_tgsi_visitor::merge_registers(void) } } - rename_temp_registers(num_renames, renames); + rename_temp_registers(renames); ralloc_free(renames); ralloc_free(last_reads); ralloc_free(first_writes); @@ -5502,7 +5501,6 @@ glsl_to_tgsi_visitor::renumber_registers(void) int new_index = 0; int *first_writes = ralloc_array(mem_ctx, int, this->ne
[Mesa-dev] [PATCH v8 2/7] mesa/st: glsl_to_tgsi move some helper classes to extra files
struct tgsi_opcode_info *info; -}; - class variable_storage { DECLARE_RZALLOC_CXX_OPERATORS(variable_storage) @@ -397,11 +151,6 @@ find_array_type(struct inout_decl *decls, unsigned count, unsigned array_id) return GLSL_TYPE_ERROR; } -struct rename_reg_pair { - bool valid; - int new_reg; -}; - struct glsl_to_tgsi_visitor : public ir_visitor { public: glsl_to_tgsi_visitor(); @@ -605,7 +354,7 @@ fail_link(struct gl_shader_program *prog, const char *fmt, ...) prog->data->LinkStatus = linking_failure; } -static int +int swizzle_for_size(int size) { static const int size_swizzles[4] = { @@ -619,40 +368,6 @@ swizzle_for_size(int size) return size_swizzles[size - 1]; } -static bool -is_resource_instruction(unsigned opcode) -{ - switch (opcode) { - case TGSI_OPCODE_RESQ: - case TGSI_OPCODE_LOAD: - case TGSI_OPCODE_ATOMUADD: - case TGSI_OPCODE_ATOMXCHG: - case TGSI_OPCODE_ATOMCAS: - case TGSI_OPCODE_ATOMAND: - case TGSI_OPCODE_ATOMOR: - case TGSI_OPCODE_ATOMXOR: - case TGSI_OPCODE_ATOMUMIN: - case TGSI_OPCODE_ATOMUMAX: - case TGSI_OPCODE_ATOMIMIN: - case TGSI_OPCODE_ATOMIMAX: - return true; - default: - return false; - } -} - -static unsigned -num_inst_dst_regs(const glsl_to_tgsi_instruction *op) -{ - return op->info->num_dst; -} - -static unsigned -num_inst_src_regs(const glsl_to_tgsi_instruction *op) -{ - return op->info->is_tex || is_resource_instruction(op->op) ? - op->info->num_src - 1 : op->info->num_src; -} glsl_to_tgsi_instruction * glsl_to_tgsi_visitor::emit_asm(ir_instruction *ir, unsigned op, diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp new file mode 100644 index 00..0075ae8676 --- /dev/null +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_private.cpp @@ -0,0 +1,196 @@ +/* + * Copyright © 2010 Intel Corporation + * Copyright © 2011 Bryan Cain + * Copyright © 2017 Gert Wollny + * + * 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 (including the next + * paragraph) 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 "st_glsl_to_tgsi_private.h" +#include +#include + +static int swizzle_for_type(const glsl_type *type, int component = 0) +{ + unsigned num_elements = 4; + + if (type) { + type = type->without_array(); + if (type->is_scalar() || type->is_vector() || type->is_matrix()) + num_elements = type->vector_elements; + } + + int swizzle = swizzle_for_size(num_elements); + assert(num_elements + component <= 4); + + swizzle += component * MAKE_SWIZZLE4(1, 1, 1, 1); + return swizzle; +} + +st_src_reg::st_src_reg(gl_register_file file, int index, const glsl_type *type, + int component, unsigned array_id) +{ + assert(file != PROGRAM_ARRAY || array_id != 0); + this->file = file; + this->index = index; + this->swizzle = swizzle_for_type(type, component); + this->negate = 0; + this->abs = 0; + this->index2D = 0; + this->type = type ? type->base_type : GLSL_TYPE_ERROR; + this->reladdr = NULL; + this->reladdr2 = NULL; + this->has_index2 = false; + this->double_reg2 = false; + this->array_id = array_id; + this->is_double_vertex_input = false; +} + +st_src_reg::st_src_reg(gl_register_file file, int index, enum glsl_base_type type) +{ + assert(file != PROGRAM_ARRAY); /* need array_id > 0 */ + this->type = type; + this->file = file; + this->index = index; + this->index2D = 0; + this->swizzle = SWIZZLE_XYZW; + this->negate = 0; + this->abs = 0; + this->reladdr = NULL; + this->reladdr2 = NULL; + this->has_index2 = false; + this->double_reg2 = false; + this->array_id = 0; + this->is_double_vertex_input = false; +} + +st_src_reg::st_src_reg(gl_register_file file, int index, enum glsl_base_ty
[Mesa-dev] [PATCH v8 3/7] mesa/st: glsl_to_tgsi: implement new temporary register lifetime tracker
This patch adds a class for tracking the life times of temporary registers in the glsl to tgsi translation. The algorithm runs in three steps: First, in order to minimize the number of needed memory allocations the program is scanned to evaluate the number of scopes. Then, the program is scanned second time to record the important register access time points: first and last reads and writes and their link to the execution scope (loop, if/else branch, switch case). In the third step for each register the actual minimal life time is evaluated. In addition, when compiled in debug mode (i.e. NDEBUG is not defined) the shaders and estimated temporary life times can be logged to stderr by setting the environment variable GLSL_TO_TGSI_RENAME_DEBUG. --- .../state_tracker/st_glsl_to_tgsi_temprename.cpp | 881 + .../state_tracker/st_glsl_to_tgsi_temprename.h | 55 ++ 2 files changed, 936 insertions(+) create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp create mode 100644 src/mesa/state_tracker/st_glsl_to_tgsi_temprename.h diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp new file mode 100644 index 00..46a707ec74 --- /dev/null +++ b/src/mesa/state_tracker/st_glsl_to_tgsi_temprename.cpp @@ -0,0 +1,881 @@ +/* + * Copyright © 2017 Gert Wollny + * + * 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 (including the next + * paragraph) 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 "st_glsl_to_tgsi_temprename.h" +#include +#include +#include +#include +#include + +/* std::sort is significantly faster than qsort */ +#define USE_STL_SORT +#ifdef USE_STL_SORT +#include +#endif + +#ifndef NDEBUG +#include +#include +#include +#include +using std::cerr; +using std::setw; +#endif + +using std::numeric_limits; + +/* Without c++11 define the nullptr for forward-compatibility + * and better readibility */ +#if __cplusplus < 201103L +#define nullptr 0 +#endif + +#ifndef NDEBUG +/* Helper function to check whether we want to seen debugging output */ +static inline bool is_debug_enabled () +{ + static int debug_enabled = -1; + if (debug_enabled < 0) + debug_enabled = env_var_as_boolean("GLSL_TO_TGSI_RENAME_DEBUG", false); + return debug_enabled > 0; +} +#define RENAME_DEBUG(X) if (is_debug_enabled()) do { X; } while (false); +#else +#define RENAME_DEBUG(X) +#endif + +namespace { + +enum prog_scope_type { + outer_scope, /* Outer program scope */ + loop_body, /* Inside a loop */ + if_branch, /* Inside if branch */ + else_branch, /* Inside else branch */ + switch_body, /* Inside switch statmenet */ + switch_case_branch,/* Inside switch case statmenet */ + switch_default_branch, /* Inside switch default statmenet */ + undefined_scope +}; + +class prog_scope { +public: + prog_scope(prog_scope *parent, prog_scope_type type, int id, + int depth, int begin); + + prog_scope_type type() const; + prog_scope *parent() const; + int nesting_depth() const; + int id() const; + int end() const; + int begin() const; + int loop_break_line() const; + + const prog_scope *in_ifelse_scope() const; + const prog_scope *in_switchcase_scope() const; + const prog_scope *innermost_loop() const; + const prog_scope *outermost_loop() const; + const prog_scope *enclosing_conditional() const; + + bool is_loop() const; + bool is_in_loop() const; + bool is_conditional() const; + bool is_conditional_in_loop() const; + + bool break_is_for_switchcase() const; + bool contains_range_of(const prog_scope& other) const; + const st_src_reg *switch_register() const; + + void set_end(int end); + void set_loop_break_line(int line); + +private: + prog_scope_type scope_type; + int scope_id; + int scope_nesting_depth; + int scope_begin; + i
[Mesa-dev] [PATCH 00/23] Silence some warnings that show up when compiling gallium/aux/util
Hello all, this might be a bit over-zealos, but I usually prefer to compile with -Wall -Wextra, and with mesa this gives many warnings. This series silences most of the warnings that are issued when compiling the C files in gallium/aux/util. Most fixes relate to -Wunused-param and are silenced by simply decorating the according parameters with "UNUSED" or "MAYBE_UNUSED" and disabling this warning would probably be of no consequence, but some other warnings might actually point to possible bugs (e.g. -Wunused-result, patch 3 and -Wsign-compare patch 4). many thanks for any comments and best regards, Gert -- Submitter has no write access to mesa git. Gert Wollny (23): gallium/aux/util/u_blitter.c: Silence some warnings gallium/aux/util/u_debug_describe.c: Silence an -Wunused-param warning gallium/aux/util/u_debug_stack.c: Silence -Wunused-result warning gallium/aux/util/u_dump_defines.c: Fix -Wcompare-unsigned warning gallium/aux/util/u_dump_state.c: Fix two -Wunused-paramter warnings gallium/aux/util/u_format.c: Fix one -Wunused-param warning gallium/aux/util/u_format_etc.c: Fix eight -Wunused-param warnings gallium/aux/util/u_format_latc.c: Fix various -Wunused-param warnings, gallium/aux/util/u_format_other.c: Fix various -Wunused-param warnings. gallium/aux/util/u_format_rgtc.c: Fix a number of -Wunused-param warnings gallium/aux/util/u_format_yuv.c: Fix a number of -Wunused-param warnings. gallium/aux/util/u_mm.c: Fix one -Wparam-unused warning. gallium/aux/util/u_pstipple.c: Fix one -Wsign-compare warning in ?: construct. gallium/aux/util/u_surface.c: Silence a -Wsign-compare warning. gallium/aux/util/u_threaded_context.c: Fix some -Wunused-param warnings. gallium/aux/util/u_transfer.c: Fix some -Wunused-param warnings. mesa/main/framebuffer.h: Fix one -Wsign-compare warning in ?: construct. mesa/main/texcompress_s3tc_tmp.h: Fix two -Wparam-unused warnings. src/util/simple_mtx.h: Fix two -Wunused-param warnings. gallium/aux/util/u_blit.c: Fix -Wunused-param warnings gallium/aux/util/u_debug_refcnt.h: Fix -Wunused-param warnings gallium/aux/os/os_thread.h: Silence -Wunused-param. gallium/aux/util/u_async_debug.c: Fix -Wtype-limits warning. src/gallium/auxiliary/os/os_thread.h| 2 + src/gallium/auxiliary/util/u_async_debug.c | 2 +- src/gallium/auxiliary/util/u_blit.c | 2 +- src/gallium/auxiliary/util/u_blitter.c | 10 +- src/gallium/auxiliary/util/u_debug_describe.c | 2 +- src/gallium/auxiliary/util/u_debug_refcnt.h | 4 +- src/gallium/auxiliary/util/u_debug_stack.c | 6 +- src/gallium/auxiliary/util/u_dump_defines.c | 2 +- src/gallium/auxiliary/util/u_dump_state.c | 4 +- src/gallium/auxiliary/util/u_format.c | 2 + src/gallium/auxiliary/util/u_format_etc.c | 8 +- src/gallium/auxiliary/util/u_format_latc.c | 24 ++- src/gallium/auxiliary/util/u_format_other.c | 34 ++-- src/gallium/auxiliary/util/u_format_rgtc.c | 26 ++- src/gallium/auxiliary/util/u_format_yuv.c | 232 src/gallium/auxiliary/util/u_mm.c | 2 +- src/gallium/auxiliary/util/u_pstipple.c | 2 +- src/gallium/auxiliary/util/u_surface.c | 2 +- src/gallium/auxiliary/util/u_threaded_context.c | 19 +- src/gallium/auxiliary/util/u_transfer.c | 20 +- src/mesa/main/framebuffer.h | 2 +- src/mesa/main/texcompress_s3tc_tmp.h| 4 +- src/util/simple_mtx.h | 4 +- 23 files changed, 222 insertions(+), 193 deletions(-) -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 03/23] gallium/aux/util/u_debug_stack.c: Silence -Wunused-result warning
asprintf is decorated with the attrbute "warn_unused_result", and if the function call fails, the pointer "temp" will be undefined, but since it is used later it should contain some usable value. Test return value of asprintf and assign some save value to "temp" if the call failed. --- src/gallium/auxiliary/util/u_debug_stack.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/gallium/auxiliary/util/u_debug_stack.c b/src/gallium/auxiliary/util/u_debug_stack.c index 7013807b6b..f12ef73e91 100644 --- a/src/gallium/auxiliary/util/u_debug_stack.c +++ b/src/gallium/auxiliary/util/u_debug_stack.c @@ -90,9 +90,9 @@ symbol_name_cached(unw_cursor_t *cursor, unw_proc_info_t *pip) procname[1] = 0; } - asprintf(&name, "%s%s", procname, ret == -UNW_ENOMEM ? "..." : ""); - - util_hash_table_set(symbols_hash, addr, (void*)name); + if (asprintf(&name, "%s%s", procname, ret == -UNW_ENOMEM ? "..." : "") == -1) + name = "??"; + util_hash_table_set(symbols_hash, addr, (void*)name); } mtx_unlock(&symbols_mutex); -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 07/23] gallium/aux/util/u_format_etc.c: Fix eight -Wunused-param warnings
Decorate the parameters accordingly with "UNUSED". --- src/gallium/auxiliary/util/u_format_etc.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/gallium/auxiliary/util/u_format_etc.c b/src/gallium/auxiliary/util/u_format_etc.c index 63e03ff5cc..8da933cf3d 100644 --- a/src/gallium/auxiliary/util/u_format_etc.c +++ b/src/gallium/auxiliary/util/u_format_etc.c @@ -17,7 +17,9 @@ util_format_etc1_rgb8_unpack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, } void -util_format_etc1_rgb8_pack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, const uint8_t *src_row, unsigned src_stride, unsigned width, unsigned height) +util_format_etc1_rgb8_pack_rgba_8unorm(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) { assert(0); } @@ -57,7 +59,9 @@ util_format_etc1_rgb8_unpack_rgba_float(float *dst_row, unsigned dst_stride, con } void -util_format_etc1_rgb8_pack_rgba_float(uint8_t *dst_row, unsigned dst_stride, const float *src_row, unsigned src_stride, unsigned width, unsigned height) +util_format_etc1_rgb8_pack_rgba_float(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const float *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) { assert(0); } -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 01/23] gallium/aux/util/u_blitter.c: Silence some warnings
* Annotate three parameters that are not used in release mode. * explicitely convert an int to unsigned in an ?: construct. --- src/gallium/auxiliary/util/u_blitter.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/gallium/auxiliary/util/u_blitter.c b/src/gallium/auxiliary/util/u_blitter.c index 72e22e7d82..de019e4b9b 100644 --- a/src/gallium/auxiliary/util/u_blitter.c +++ b/src/gallium/auxiliary/util/u_blitter.c @@ -550,7 +550,7 @@ void util_blitter_unset_running_flag(struct blitter_context *blitter) blitter->pipe->set_active_query_state(blitter->pipe, true); } -static void blitter_check_saved_vertex_states(struct blitter_context_priv *ctx) +static void blitter_check_saved_vertex_states(struct blitter_context_priv *ctx MAYBE_UNUSED) { assert(ctx->base.saved_vs != INVALID_PTR); assert(!ctx->has_geometry_shader || ctx->base.saved_gs != INVALID_PTR); @@ -616,7 +616,7 @@ void util_blitter_restore_vertex_states(struct blitter_context *blitter) ctx->base.saved_rs_state = INVALID_PTR; } -static void blitter_check_saved_fragment_states(struct blitter_context_priv *ctx) +static void blitter_check_saved_fragment_states(struct blitter_context_priv *ctx MAYBE_UNUSED) { assert(ctx->base.saved_fs != INVALID_PTR); assert(ctx->base.saved_dsa_state != INVALID_PTR); @@ -655,7 +655,7 @@ void util_blitter_restore_fragment_states(struct blitter_context *blitter) pipe->set_viewport_states(pipe, 0, 1, &ctx->base.saved_viewport); } -static void blitter_check_saved_fb_state(struct blitter_context_priv *ctx) +static void blitter_check_saved_fb_state(struct blitter_context_priv *ctx MAYBE_UNUSED) { assert(ctx->base.saved_fb_state.nr_cbufs != ~0u); } @@ -691,7 +691,7 @@ void util_blitter_restore_fb_state(struct blitter_context *blitter) util_unreference_framebuffer_state(&ctx->base.saved_fb_state); } -static void blitter_check_saved_textures(struct blitter_context_priv *ctx) +static void blitter_check_saved_textures(struct blitter_context_priv *ctx MAYBE_UNUSED) { assert(ctx->base.saved_num_sampler_states != ~0u); assert(ctx->base.saved_num_sampler_views != ~0u); @@ -1488,7 +1488,7 @@ void util_blitter_default_src_texture(struct blitter_context *blitter, src_templ->u.tex.first_layer = 0; src_templ->u.tex.last_layer = src->target == PIPE_TEXTURE_3D ? u_minify(src->depth0, srclevel) - 1 - : src->array_size - 1; + : (unsigned)(src->array_size - 1); src_templ->swizzle_r = PIPE_SWIZZLE_X; src_templ->swizzle_g = PIPE_SWIZZLE_Y; src_templ->swizzle_b = PIPE_SWIZZLE_Z; -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 04/23] gallium/aux/util/u_dump_defines.c: Fix -Wsign-compare warning
u_bit_scan may return -1 that then may be interpreted as (unsigned)-1 in the following comparison, since num_names is unsigned. Convert the latter to be int as well. --- src/gallium/auxiliary/util/u_dump_defines.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/auxiliary/util/u_dump_defines.c b/src/gallium/auxiliary/util/u_dump_defines.c index e431cd969b..50dfa37bab 100644 --- a/src/gallium/auxiliary/util/u_dump_defines.c +++ b/src/gallium/auxiliary/util/u_dump_defines.c @@ -99,7 +99,7 @@ util_dump_flags_continuous(FILE *stream, unsigned value, unsigned num_names, while (value) { int i = u_bit_scan(&value); - if (i >= num_names || !names[i]) + if (i >= (int)num_names || !names[i]) unknown |= 1u << i; if (!first) fputs("|", stream); -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 08/23] gallium/aux/util/u_format_latc.c: Fix various -Wunused-param warnings,
Decorate the unused params with "UNUSED". --- src/gallium/auxiliary/util/u_format_latc.c | 24 +--- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/src/gallium/auxiliary/util/u_format_latc.c b/src/gallium/auxiliary/util/u_format_latc.c index 31d72b5a0a..ce69371358 100644 @@ -100,19 +100,24 @@ util_format_latc1_unorm_fetch_rgba_float(float *dst, const uint8_t *src, unsigne } void -util_format_latc1_snorm_fetch_rgba_8unorm(uint8_t *dst, const uint8_t *src, unsigned i, unsigned j) +util_format_latc1_snorm_fetch_rgba_8unorm(uint8_t *dst UNUSED, const uint8_t *src UNUSED, + unsigned i UNUSED, unsigned j UNUSED) { fprintf(stderr,"%s\n", __func__); } void -util_format_latc1_snorm_unpack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, const uint8_t *src_row, unsigned src_stride, unsigned width, unsigned height) +util_format_latc1_snorm_unpack_rgba_8unorm(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) { fprintf(stderr,"%s\n", __func__); } void -util_format_latc1_snorm_pack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, const uint8_t *src_row, unsigned src_stride, unsigned width, unsigned height) +util_format_latc1_snorm_pack_rgba_8unorm(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) { fprintf(stderr,"%s\n", __func__); } @@ -231,19 +236,24 @@ util_format_latc2_unorm_fetch_rgba_float(float *dst, const uint8_t *src, unsigne void -util_format_latc2_snorm_fetch_rgba_8unorm(uint8_t *dst, const uint8_t *src, unsigned i, unsigned j) +util_format_latc2_snorm_fetch_rgba_8unorm(uint8_t *dst UNUSED, const uint8_t *src UNUSED, + unsigned i UNUSED, unsigned j UNUSED) { fprintf(stderr,"%s\n", __func__); } void -util_format_latc2_snorm_unpack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, const uint8_t *src_row, unsigned src_stride, unsigned width, unsigned height) +util_format_latc2_snorm_unpack_rgba_8unorm(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) { fprintf(stderr,"%s\n", __func__); } void -util_format_latc2_snorm_pack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, const uint8_t *src_row, unsigned src_stride, unsigned width, unsigned height) +util_format_latc2_snorm_pack_rgba_8unorm(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) { fprintf(stderr,"%s\n", __func__); } -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 02/23] gallium/aux/util/u_debug_describe.c: Silence an -Wunused-param warning
* Annotate the unused parameter. --- src/gallium/auxiliary/util/u_debug_describe.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/auxiliary/util/u_debug_describe.c b/src/gallium/auxiliary/util/u_debug_describe.c index f428d22d20..9a8e0cc392 100644 --- a/src/gallium/auxiliary/util/u_debug_describe.c +++ b/src/gallium/auxiliary/util/u_debug_describe.c @@ -30,7 +30,7 @@ #include "util/u_string.h" void -debug_describe_reference(char* buf, const struct pipe_reference*ptr) +debug_describe_reference(char* buf, const struct pipe_reference*ptr UNUSED) { strcpy(buf, "pipe_object"); } -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 05/23] gallium/aux/util/u_dump_state.c: Fix two -Wunused-paramter warnings
--- src/gallium/auxiliary/util/u_dump_state.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/gallium/auxiliary/util/u_dump_state.c b/src/gallium/auxiliary/util/u_dump_state.c index 2725d50cd9..512fb677d3 100644 --- a/src/gallium/auxiliary/util/u_dump_state.c +++ b/src/gallium/auxiliary/util/u_dump_state.c @@ -112,7 +112,7 @@ util_dump_array_end(FILE *stream) } static void -util_dump_elem_begin(FILE *stream) +util_dump_elem_begin(FILE *stream UNUSED) { } @@ -123,7 +123,7 @@ util_dump_elem_end(FILE *stream) } static void -util_dump_struct_begin(FILE *stream, const char *name) +util_dump_struct_begin(FILE *stream, const char *name UNUSED) { fputs("{", stream); } -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 06/23] gallium/aux/util/u_format.c: Fix one -Wunused-param warning
This warning was issued only in release mode. Fix it by fake-using the parameter. --- src/gallium/auxiliary/util/u_format.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/gallium/auxiliary/util/u_format.c b/src/gallium/auxiliary/util/u_format.c index 0fc3231654..369b4c3126 100644 --- a/src/gallium/auxiliary/util/u_format.c +++ b/src/gallium/auxiliary/util/u_format.c @@ -249,6 +249,8 @@ util_format_is_supported(enum pipe_format format, unsigned bind) util_format_is_float(format)) { return FALSE; } +#else + (void)bind; #endif return TRUE; -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 09/23] gallium/aux/util/u_format_other.c: Fix various -Wunused-param warnings.
Decorate the unused params with "UNUSED". --- src/gallium/auxiliary/util/u_format_other.c | 34 ++--- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/gallium/auxiliary/util/u_format_other.c b/src/gallium/auxiliary/util/u_format_other.c index c53bf550a6..188b45b0da 100644 --- a/src/gallium/auxiliary/util/u_format_other.c +++ b/src/gallium/auxiliary/util/u_format_other.c @@ -75,7 +75,7 @@ util_format_r9g9b9e5_float_pack_rgba_float(uint8_t *dst_row, unsigned dst_stride void util_format_r9g9b9e5_float_fetch_rgba_float(float *dst, const uint8_t *src, - unsigned i, unsigned j) + unsigned i UNUSED, unsigned j UNUSED) { uint32_t value = util_cpu_to_le32(*(const uint32_t *)src); rgb9e5_to_float3(value, dst); @@ -178,7 +178,7 @@ util_format_r11g11b10_float_pack_rgba_float(uint8_t *dst_row, unsigned dst_strid void util_format_r11g11b10_float_fetch_rgba_float(float *dst, const uint8_t *src, - unsigned i, unsigned j) + unsigned i UNUSED, unsigned j UNUSED) { uint32_t value = util_cpu_to_le32(*(const uint32_t *)src); r11g11b10f_to_float3(value, dst); @@ -239,44 +239,44 @@ util_format_r11g11b10_float_pack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stri void -util_format_r1_unorm_unpack_rgba_float(float *dst_row, unsigned dst_stride, - const uint8_t *src_row, unsigned src_stride, - unsigned width, unsigned height) +util_format_r1_unorm_unpack_rgba_float(float *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) { } void -util_format_r1_unorm_pack_rgba_float(uint8_t *dst_row, unsigned dst_stride, -const float *src_row, unsigned src_stride, -unsigned width, unsigned height) +util_format_r1_unorm_pack_rgba_float(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, +const float *src_row UNUSED, unsigned src_stride UNUSED, +unsigned width UNUSED, unsigned height UNUSED) { } void -util_format_r1_unorm_fetch_rgba_float(float *dst, const uint8_t *src, - unsigned i, unsigned j) +util_format_r1_unorm_fetch_rgba_float(float *dst UNUSED, const uint8_t *src UNUSED, + unsigned i UNUSED, unsigned j UNUSED) { } void -util_format_r1_unorm_unpack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, - const uint8_t *src_row, unsigned src_stride, - unsigned width, unsigned height) +util_format_r1_unorm_unpack_rgba_8unorm(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) { } void -util_format_r1_unorm_pack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, - const uint8_t *src_row, unsigned src_stride, - unsigned width, unsigned height) +util_format_r1_unorm_pack_rgba_8unorm(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) { } @@ -407,7 +407,7 @@ util_format_r8g8bx_snorm_pack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, void util_format_r8g8bx_snorm_fetch_rgba_float(float *dst, const uint8_t *src, - unsigned i, unsigned j) + unsigned i UNUSED, unsigned j UNUSED) { uint16_t value = util_cpu_to_le16(*(const uint16_t *)src); int16_t r, g; -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 13/23] gallium/aux/util/u_pstipple.c: Fix one -Wsign-compare warning in ?: construct.
Silence the warning by making the conversion to int explicit. --- src/gallium/auxiliary/util/u_pstipple.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/gallium/auxiliary/util/u_pstipple.c b/src/gallium/auxiliary/util/u_pstipple.c index ae4cfa1d76..77b0458dc0 100644 --- a/src/gallium/auxiliary/util/u_pstipple.c +++ b/src/gallium/auxiliary/util/u_pstipple.c @@ -297,7 +297,7 @@ pstip_transform_prolog(struct tgsi_transform_context *ctx) ctx->emit_declaration(ctx, &decl); } - sampIdx = pctx->hasFixedUnit ? pctx->fixedUnit : pctx->freeSampler; + sampIdx = pctx->hasFixedUnit ? (int)pctx->fixedUnit : pctx->freeSampler; /* declare new sampler */ tgsi_transform_sampler_decl(ctx, sampIdx); -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 10/23] gallium/aux/util/u_format_rgtc.c: Fix a number of -Wunused-param warnings
Decorate the params accordingly with "UNUSED". --- src/gallium/auxiliary/util/u_format_rgtc.c | 26 ++ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/src/gallium/auxiliary/util/u_format_rgtc.c b/src/gallium/auxiliary/util/u_format_rgtc.c index 1596917435..82ddcb0183 100644 --- a/src/gallium/auxiliary/util/u_format_rgtc.c +++ b/src/gallium/auxiliary/util/u_format_rgtc.c @@ -63,8 +63,8 @@ util_format_rgtc1_unorm_unpack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride } void -util_format_rgtc1_unorm_pack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, const uint8_t *src_row, -unsigned src_stride, unsigned width, unsigned height) +util_format_rgtc1_unorm_pack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, const uint8_t *src_row, + unsigned src_stride, unsigned width, unsigned height) { const unsigned bw = 4, bh = 4, bytes_per_block = 8; unsigned x, y, i, j; @@ -144,19 +144,24 @@ util_format_rgtc1_unorm_fetch_rgba_float(float *dst, const uint8_t *src, unsigne } void -util_format_rgtc1_snorm_fetch_rgba_8unorm(uint8_t *dst, const uint8_t *src, unsigned i, unsigned j) +util_format_rgtc1_snorm_fetch_rgba_8unorm(uint8_t *dst UNUSED, const uint8_t *src UNUSED, + unsigned i UNUSED, unsigned j UNUSED) { fprintf(stderr,"%s\n", __func__); } void -util_format_rgtc1_snorm_unpack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, const uint8_t *src_row, unsigned src_stride, unsigned width, unsigned height) +util_format_rgtc1_snorm_unpack_rgba_8unorm(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) { fprintf(stderr,"%s\n", __func__); } void -util_format_rgtc1_snorm_pack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, const uint8_t *src_row, unsigned src_stride, unsigned width, unsigned height) +util_format_rgtc1_snorm_pack_rgba_8unorm(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) { fprintf(stderr,"%s\n", __func__); } @@ -350,19 +355,24 @@ util_format_rgtc2_unorm_fetch_rgba_float(float *dst, const uint8_t *src, unsigne void -util_format_rgtc2_snorm_fetch_rgba_8unorm(uint8_t *dst, const uint8_t *src, unsigned i, unsigned j) +util_format_rgtc2_snorm_fetch_rgba_8unorm(uint8_t *dst UNUSED, const uint8_t *src UNUSED, + unsigned i UNUSED, unsigned j UNUSED) { fprintf(stderr,"%s\n", __func__); } void -util_format_rgtc2_snorm_unpack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, const uint8_t *src_row, unsigned src_stride, unsigned width, unsigned height) +util_format_rgtc2_snorm_unpack_rgba_8unorm(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) { fprintf(stderr,"%s\n", __func__); } void -util_format_rgtc2_snorm_pack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, const uint8_t *src_row, unsigned src_stride, unsigned width, unsigned height) +util_format_rgtc2_snorm_pack_rgba_8unorm(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) { fprintf(stderr,"%s\n", __func__); } -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 11/23] gallium/aux/util/u_format_yuv.c: Fix a number of -Wunused-param warnings.
Decorate the params accordingly with "UNUSED". --- src/gallium/auxiliary/util/u_format_yuv.c | 232 +++--- 1 file changed, 116 insertions(+), 116 deletions(-) diff --git a/src/gallium/auxiliary/util/u_format_yuv.c b/src/gallium/auxiliary/util/u_format_yuv.c index 55f8ad68ec..3ae32406c5 100644 --- a/src/gallium/auxiliary/util/u_format_yuv.c +++ b/src/gallium/auxiliary/util/u_format_yuv.c @@ -246,7 +246,7 @@ util_format_r8g8_b8g8_unorm_pack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stri void util_format_r8g8_b8g8_unorm_fetch_rgba_float(float *dst, const uint8_t *src, -unsigned i, unsigned j) +unsigned i, unsigned j MAYBE_UNUSED) { assert(i < 2); assert(j < 1); @@ -466,7 +466,7 @@ util_format_g8r8_g8b8_unorm_pack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stri void util_format_g8r8_g8b8_unorm_fetch_rgba_float(float *dst, const uint8_t *src, -unsigned i, unsigned j) +unsigned i, unsigned j MAYBE_UNUSED) { assert(i < 2); assert(j < 1); @@ -682,7 +682,7 @@ util_format_uyvy_pack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, void util_format_uyvy_fetch_rgba_float(float *dst, const uint8_t *src, - unsigned i, unsigned j) + unsigned i, unsigned j MAYBE_UNUSED) { uint8_t y, u, v; @@ -903,7 +903,7 @@ util_format_yuyv_pack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, void util_format_yuyv_fetch_rgba_float(float *dst, const uint8_t *src, - unsigned i, unsigned j) + unsigned i, unsigned j MAYBE_UNUSED) { uint8_t y, u, v; @@ -921,164 +921,164 @@ util_format_yuyv_fetch_rgba_float(float *dst, const uint8_t *src, /* XXX: Stubbed for now */ void -util_format_yv12_unpack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, - const uint8_t *src_row, unsigned src_stride, - unsigned width, unsigned height) {} +util_format_yv12_unpack_rgba_8unorm(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) {} void -util_format_yv12_pack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, - const uint8_t *src_row, unsigned src_stride, - unsigned width, unsigned height) {} +util_format_yv12_pack_rgba_8unorm(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) {} void -util_format_yv12_unpack_rgba_float(float *dst_row, unsigned dst_stride, - const uint8_t *src_row, unsigned src_stride, - unsigned width, unsigned height) {} +util_format_yv12_unpack_rgba_float(float *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) {} void -util_format_yv12_pack_rgba_float(uint8_t *dst_row, unsigned dst_stride, - const float *src_row, unsigned src_stride, - unsigned width, unsigned height) {} +util_format_yv12_pack_rgba_float(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const float *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) {} void -util_format_yv12_fetch_rgba_float(float *dst, const uint8_t *src, - unsigned i, unsigned j) {} +util_format_yv12_fetch_rgba_float(float *dst UNUSED, const uint8_t *src UNUSED, + unsigned i UNUSED, unsigned j UNUSED) {} void -util_format_yv16_unpack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, - const uint8_t *src_row, unsigned src_stride, - unsigned width, unsigned height) {} +util_format_yv16_unpack_rgba_8unorm(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, + unsigned width UNUSED, unsigned height UNUSED) {} void -util_format_yv16_pack_rgba_8unorm(uint8_t *dst_row, unsigned dst_stride, - const uint8_t *src_row, unsigned src_stride, - unsigned width, unsigned height) {} +util_format_yv16_pack_rgba_8unorm(uint8_t *dst_row UNUSED, unsigned dst_stride UNUSED, + const uint8_t *src_row UNUSED, unsigned src_stride UNUSED, +
[Mesa-dev] [PATCH 19/23] src/util/simple_mtx.h: Fix two -Wunused-param warnings.
Decorate the parameters accordingly with "UNUSED" or "MAYBE_UNUSED" (for the param that is used in debug mode, but not in release mode). --- src/util/simple_mtx.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/util/simple_mtx.h b/src/util/simple_mtx.h index cd24b6f9eb..ed7f657a9d 100644 --- a/src/util/simple_mtx.h +++ b/src/util/simple_mtx.h @@ -61,7 +61,7 @@ typedef struct { #define _SIMPLE_MTX_INITIALIZER_NP { 0 } static inline void -simple_mtx_init(simple_mtx_t *mtx, int type) +simple_mtx_init(simple_mtx_t *mtx, int type MAYBE_UNUSED) { assert(type == mtx_plain); @@ -69,7 +69,7 @@ simple_mtx_init(simple_mtx_t *mtx, int type) } static inline void -simple_mtx_destroy(simple_mtx_t *mtx) +simple_mtx_destroy(simple_mtx_t *mtx UNUSED) { } -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 17/23] mesa/main/framebuffer.h: Fix one -Wsign-compare warning in ?: construct.
Explicitely convert on value to the target type. --- src/mesa/main/framebuffer.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/main/framebuffer.h b/src/mesa/main/framebuffer.h index bc6e7bc31a..bffa21dcea 100644 --- a/src/mesa/main/framebuffer.h +++ b/src/mesa/main/framebuffer.h @@ -93,7 +93,7 @@ static inline GLuint _mesa_geometric_samples(const struct gl_framebuffer *buffer) { return buffer->_HasAttachments ? - buffer->Visual.samples : + (GLuint)buffer->Visual.samples : buffer->DefaultGeometry._NumSamples; } -- 2.13.6 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev