On Fri, 2018-03-23 at 13:54 -0700, Ian Romanick wrote:
> From: Ian Romanick <ian.d.roman...@intel.com>
>
> A recent commit (see below) triggered some cases where conditional
> modifier propagation and dead code elimination would cause a MAD
> instruction like the following to be generated:
>
> mad.l.f0 null, ...
>
> Matt pointed out that fs_visitor::fixup_3src_null_dest() fixes cases
> like this in the scalar backend. This commit basically ports that code
> to the vec4 backend.
>
> NOTE: I have sent a couple tests to the piglit list that reproduce this
> bug *without* the commit mentioned below. This commit fixes those
> tests.
>
> Signed-off-by: Ian Romanick <ian.d.roman...@intel.com>
> Cc: Tapani Pälli <tapani.pa...@intel.com>
> Cc: Matt Turner <matts...@gmail.com>
> Cc: mesa-sta...@lists.freedesktop.org
> Fixes: ee63933a7 ("nir: Distribute binary operations with constants into
> bcsel")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105704
I need a clarification with this commit for inclusion on next 18.0 stable
release.
The commit was nominated for stable, but at the same time it has a "Fixes" tag.
The commit applies cleanly in the stable branch, but the commit it fixes
(ee63933a7) was not landed in branch.
So I'd like to know if this patch should be included in stable or not.
J.A.
> ---
> src/intel/compiler/brw_vec4.cpp | 26 ++++++++++++++++++++++++++
> src/intel/compiler/brw_vec4.h | 1 +
> 2 files changed, 27 insertions(+)
>
> diff --git a/src/intel/compiler/brw_vec4.cpp b/src/intel/compiler/brw_vec4.cpp
> index e483814..fb8ffee 100644
> --- a/src/intel/compiler/brw_vec4.cpp
> +++ b/src/intel/compiler/brw_vec4.cpp
> @@ -1945,6 +1945,30 @@ is_align1_df(vec4_instruction *inst)
> }
> }
>
> +/**
> + * Three source instruction must have a GRF/MRF destination register.
> + * ARF NULL is not allowed. Fix that up by allocating a temporary GRF.
> + */
> +void
> +vec4_visitor::fixup_3src_null_dest()
> +{
> + bool progress = false;
> +
> + foreach_block_and_inst_safe (block, vec4_instruction, inst, cfg) {
> + if (inst->is_3src(devinfo) && inst->dst.is_null()) {
> + const unsigned size_written = type_sz(inst->dst.type);
> + const unsigned num_regs = DIV_ROUND_UP(size_written, REG_SIZE);
> +
> + inst->dst = retype(dst_reg(VGRF, alloc.allocate(num_regs)),
> + inst->dst.type);
> + progress = true;
> + }
> + }
> +
> + if (progress)
> + invalidate_live_intervals();
> +}
> +
> void
> vec4_visitor::convert_to_hw_regs()
> {
> @@ -2696,6 +2720,8 @@ vec4_visitor::run()
> OPT(scalarize_df);
> }
>
> + fixup_3src_null_dest();
> +
> bool allocated_without_spills = reg_allocate();
>
> if (!allocated_without_spills) {
> diff --git a/src/intel/compiler/brw_vec4.h b/src/intel/compiler/brw_vec4.h
> index 39ce51c..71880db 100644
> --- a/src/intel/compiler/brw_vec4.h
> +++ b/src/intel/compiler/brw_vec4.h
> @@ -158,6 +158,7 @@ public:
> void opt_set_dependency_control();
> void opt_schedule_instructions();
> void convert_to_hw_regs();
> + void fixup_3src_null_dest();
>
> bool is_supported_64bit_region(vec4_instruction *inst, unsigned arg);
> bool lower_simd_width();
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
