This involves wrapping fs_live_variables in a BRW_ANALYSIS object and
hooking it up to invalidate_analysis() so it's properly invalidated.
Seems like a lot of churn but it's fairly straightforward. The
fs_visitor invalidate_ and calculate_live_intervals() methods are no
longer necessary after this change.
Reviewed-by: Matt Turner <mattst88@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4012>
This could be improved somewhat with additional validation of the
calculated live in/out sets and by checking that the calculated live
intervals are minimal (which isn't strictly necessary to guarantee the
correctness of the program). This should be good enough though to
catch accidental use of stale liveness results due to missing or
incorrect analysis invalidation.
Reviewed-by: Matt Turner <mattst88@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4012>
This removes the dependency of fs_live_variables on fs_visitor. The
IR analysis framework requires the analysis result to be constructible
with a single argument -- The second argument was redundant anyway.
Reviewed-by: Matt Turner <mattst88@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4012>
This moves the following methods that are currently defined in
fs_visitor (even though they are side products of the liveness
analysis computation) and are already implemented in
brw_fs_live_variables.cpp:
> bool virtual_grf_interferes(int a, int b) const;
> int *virtual_grf_start;
> int *virtual_grf_end;
It makes sense for them to be part of the fs_live_variables object,
because they have the same lifetime as other liveness analysis results
and because this will allow some extra validation to happen wherever
they are accessed in order to make sure that we only ever use
up-to-date liveness analysis results.
This shortens the virtual_grf prefix in order to compensate for the
slightly increased lexical overhead from the live_intervals pointer
dereference.
Reviewed-by: Matt Turner <mattst88@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4012>
brw_fs.h (in particular fs_visitor) is logically a user of the live
variables analysis pass, not the other way around.
brw_fs_live_variables.h requires the definition of some FS IR data
structures to compile, but those can be obtained directly from
brw_ir_fs.h without including brw_fs.h. The dependency of
fs_live_variables on fs_visitor is rather accidental and will be
removed in a future commit, a forward declaration is enough for the
moment.
Reviewed-by: Matt Turner <mattst88@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4012>
When this commit was originally written, these two structures had the
exact same name. Subsequently in commit 12a8f2616a (intel/compiler:
Fix C++ one definition rule violations) they were renamed.
Original commit message:
> These two structures have exactly the same name which prevents the two
> files from being included at the same time and could cause serious
> trouble in the future if it ever leads to a (silent) violation of the
> C++ one definition rule.
Reviewed-by: Matt Turner <mattst88@gmail.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/4012>
When building with "-flto" brw::block_data definitions
were colliding.
Signed-off-by: Danylo Piliaiev <danylo.piliaiev@globallogic.com>
Reviewed-by: Matt Turner <mattst88@gmail.com>
Currently the liveness analysis pass would extend a live interval up
to the top of the program when no unconditional and complete
definition of the variable is found that dominates all of its uses.
This can lead to a serious performance problem in shaders containing
many partial writes, like scalar arithmetic, FP64 and soon FP16
operations. The number of oversize live intervals in such workloads
can cause the compilation time of the shader to explode because of the
worse than quadratic behavior of the register allocator and scheduler
when running out of registers, and it can also cause the running time
of the shader to explode due to the amount of spilling it leads to,
which is orders of magnitude slower than GRF memory.
This patch fixes it by computing the intersection of our current live
intervals with the subset of the program that can possibly be reached
from any definition of the variable. Extending the storage allocation
of the variable beyond that is pretty useless because its value is
guaranteed to be undefined at a point that cannot be reached from any
definition.
According to Jason, this improves performance of the subgroup Vulkan
CTS tests significantly (e.g. the runtime of the dvec4 broadcast test
improves by nearly 50x).
No significant change in the running time of shader-db (with 5%
statistical significance).
shader-db results on IVB:
total cycles in shared programs: 61108780 -> 60932856 (-0.29%)
cycles in affected programs: 16335482 -> 16159558 (-1.08%)
helped: 5121
HURT: 4347
total spills in shared programs: 1309 -> 1288 (-1.60%)
spills in affected programs: 249 -> 228 (-8.43%)
helped: 3
HURT: 0
total fills in shared programs: 1652 -> 1597 (-3.33%)
fills in affected programs: 262 -> 207 (-20.99%)
helped: 4
HURT: 0
LOST: 2
GAINED: 209
shader-db results on BDW:
total cycles in shared programs: 67617262 -> 67361220 (-0.38%)
cycles in affected programs: 23397142 -> 23141100 (-1.09%)
helped: 8045
HURT: 6488
total spills in shared programs: 1456 -> 1252 (-14.01%)
spills in affected programs: 465 -> 261 (-43.87%)
helped: 3
HURT: 0
total fills in shared programs: 1720 -> 1465 (-14.83%)
fills in affected programs: 471 -> 216 (-54.14%)
helped: 4
HURT: 0
LOST: 2
GAINED: 162
shader-db results on SKL:
total cycles in shared programs: 65436248 -> 65245186 (-0.29%)
cycles in affected programs: 22560936 -> 22369874 (-0.85%)
helped: 8457
HURT: 6247
total spills in shared programs: 437 -> 437 (0.00%)
spills in affected programs: 0 -> 0
helped: 0
HURT: 0
total fills in shared programs: 870 -> 854 (-1.84%)
fills in affected programs: 16 -> 0
helped: 1
HURT: 0
LOST: 0
GAINED: 107
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
Mostly a dummy git mv with a couple of noticable parts:
- With the earlier header cleanups, nothing in src/intel depends
files from src/mesa/drivers/dri/i965/
- Both Autoconf and Android builds are addressed. Thanks to Mauro and
Tapani for the fixups in the latter
- brw_util.[ch] is not really compiler specific, so it's moved to i965.
v2:
- move brw_eu_defines.h instead of brw_defines.h
- remove no-longer applicable includes
- add missing vulkan/ prefix in the Android build (thanks Tapani)
v3:
- don't list brw_defines.h in src/intel/Makefile.sources (Jason)
- rebase on top of the oa patches
[Emil Velikov: commit message, various small fixes througout]
Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
2017-03-13 11:16:34 +00:00
Renamed from src/mesa/drivers/dri/i965/brw_fs_live_variables.h (Browse further)
Anuj Phogat
Ian Romanick
Francisco Jerez
Danylo Piliaiev
Emil Velikov
Jason Ekstrand