On 08/09/17 01:58, Jason Ekstrand wrote:
> On Tue, Aug 29, 2017 at 11:54 PM, Alejandro Piñeiro
> <apinhe...@igalia.com <mailto:apinhe...@igalia.com>> wrote:
>
> Although from SPIR-V point of view, rounding modes are attached to the
> operation/destination, on i965 it is a status, so we don't need to
> explicitly set the rounding mode if the one we want is already set.
>
> v2: Use a single SHADER_OPCODE_RND_MODE opcode taking an immediate
> with the rounding mode (Curro)
>
> Signed-off-by: Jose Maria Casanova Crespo <jmcasan...@igalia.com
> <mailto:jmcasan...@igalia.com>>
> Signed-off-by: Alejandro Piñeiro <apinhe...@igalia.com
> <mailto:apinhe...@igalia.com>
>
> ---
> src/intel/compiler/brw_fs.cpp | 41
> +++++++++++++++++++++++++++++++++++++++++
> src/intel/compiler/brw_fs.h | 1 +
> 2 files changed, 42 insertions(+)
>
> diff --git a/src/intel/compiler/brw_fs.cpp
> b/src/intel/compiler/brw_fs.cpp
> index 45236e3cab7..1b9ab0c0b88 100644
> --- a/src/intel/compiler/brw_fs.cpp
> +++ b/src/intel/compiler/brw_fs.cpp
> @@ -3071,6 +3071,46 @@ fs_visitor::remove_duplicate_mrf_writes()
> return progress;
> }
>
> +/**
> + * Rounding modes for conversion instructions are included for each
> + * conversion, but right now it is a state. So once it is set,
> + * we don't need to call it again for subsequent calls.
> + *
> + * This is useful for vector/matrices conversions, as setting the
> + * mode once is enough for the full vector/matrix
> + */
> +bool
> +fs_visitor::remove_extra_rounding_modes()
> +{
> + bool progress = false;
> + /* By default, the rounding mode is rte, so we can consider it
> as the
> + * starting rounding mode
> + */
> + brw_rnd_mode prev_mode = BRW_RND_MODE_RTNE;
>
>
> This needs to be reset for every block. Otherwise, you may end up
> invalidly making assumptions about the rounding mode at the start of a
> loop or on two sides of an if. This probably means you should just
> use foreach_block and foreach_inst_safe separately.
True, Jose Maria pointed me the same on a internal review, and I
remember doing the change. It seems that it got lost on one of my
development branches.
Will try to find it, if not we can reimplement it.
>
>
> + brw_rnd_mode current_mode;
>
>
> I think I'd prefer this be declared right there in the loop where it's
> used. Maybe make it const and just call it "mode"?
>
>
> +
> + foreach_block_and_inst_safe (block, fs_inst, inst, cfg) {
> + if (inst->opcode == SHADER_OPCODE_RND_MODE) {
> + assert(inst->src[0].file == BRW_IMMEDIATE_VALUE);
> +
> + current_mode = (brw_rnd_mode) inst->src[0].d;
> +
> + if (current_mode == prev_mode) {
> + inst->remove(block);
> + progress = true;
> + } else {
> + prev_mode = current_mode;
> + }
> + }
> + }
> +
> + if (progress)
> + invalidate_live_intervals();
> +
> + return progress;
> +}
> +
> +
> static void
> clear_deps_for_inst_src(fs_inst *inst, bool *deps, int first_grf,
> int grf_len)
> {
> @@ -5748,6 +5788,7 @@ fs_visitor::optimize()
> int pass_num = 0;
>
> OPT(opt_drop_redundant_mov_to_flags);
> + OPT(remove_extra_rounding_modes);
>
> do {
> progress = false;
> diff --git a/src/intel/compiler/brw_fs.h b/src/intel/compiler/brw_fs.h
> index f1ba193de7e..b9476e69edb 100644
> --- a/src/intel/compiler/brw_fs.h
> +++ b/src/intel/compiler/brw_fs.h
> @@ -150,6 +150,7 @@ public:
> bool eliminate_find_live_channel();
> bool dead_code_eliminate();
> bool remove_duplicate_mrf_writes();
> + bool remove_extra_rounding_modes();
>
> bool opt_sampler_eot();
> bool virtual_grf_interferes(int a, int b);
> --
> 2.11.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org <mailto:mesa-dev@lists.freedesktop.org>
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> <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
