[Mesa-dev] [PATCH 2/3] i965: Lower min/max after optimization on Gen4/5.

[Matt Turner]

On Thu, Feb 11, 2016 at 4:41 PM, Matt Turner <mattst88 at gmail.com> wrote:
> Gen4/5's SEL instruction cannot use conditional modifiers, so min/max
> are implemented as CMP + SEL. Handling that after optimization lets us
> CSE more.
>
> On Ironlake:
>
>    total instructions in shared programs: 6426035 -> 6422753 (-0.05%)
>    instructions in affected programs: 326604 -> 323322 (-1.00%)
>    helped: 1411
>
>    total cycles in shared programs: 129184700 -> 129101586 (-0.06%)
>    cycles in affected programs: 18950290 -> 18867176 (-0.44%)
>    helped: 2419
>    HURT: 328
> ---
>  src/mesa/drivers/dri/i965/brw_fs.cpp           | 37 +++++++++++++++++++++++++
>  src/mesa/drivers/dri/i965/brw_fs.h             |  1 +
>  src/mesa/drivers/dri/i965/brw_fs_builder.h     | 10 ++-----
>  src/mesa/drivers/dri/i965/brw_fs_nir.cpp       | 20 +++-----------
>  src/mesa/drivers/dri/i965/brw_vec4.cpp         | 38 ++++++++++++++++++++++++++
>  src/mesa/drivers/dri/i965/brw_vec4.h           |  2 ++
>  src/mesa/drivers/dri/i965/brw_vec4_builder.h   | 10 ++-----
>  src/mesa/drivers/dri/i965/brw_vec4_visitor.cpp | 14 ++--------
>  8 files changed, 88 insertions(+), 44 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_fs.cpp b/src/mesa/drivers/dri/i965/brw_fs.cpp
> index 0ce7ed1..e83f0ba 100644
> --- a/src/mesa/drivers/dri/i965/brw_fs.cpp
> +++ b/src/mesa/drivers/dri/i965/brw_fs.cpp
> @@ -3475,6 +3475,36 @@ fs_visitor::lower_integer_multiplication()
>     return progress;
>  }
>
> +bool
> +fs_visitor::lower_minmax()
> +{
> +   assert(devinfo->gen < 6);
> +
> +   bool progress = false;
> +
> +   foreach_block_and_inst_safe(block, fs_inst, inst, cfg) {
> +      const fs_builder ibld(this, block, inst);
> +
> +      if (inst->opcode == BRW_OPCODE_SEL &&
> +          inst->predicate == BRW_PREDICATE_NONE) {
> +         assert(inst->conditional_mod == BRW_CONDITIONAL_GE ||
> +                inst->conditional_mod == BRW_CONDITIONAL_L);

Ken asked at the office if this assertion is necessary. I think it is.
The PRM doesn't say anything about SEL with conditional modifiers
other than .ge or .l.