[Mesa-stable] [PATCH] i965/vec4: Fix null destination register in 3-source instructions
Tapani Pälli
tapani.palli at intel.com
Mon Mar 26 04:49:35 UTC 2018
Yes, this fixes the app where I saw this issue, thanks!
Tested-by: Tapani Pälli <tapani.palli at intel.com>
On 03/23/2018 10:54 PM, Ian Romanick wrote:
> From: Ian Romanick <ian.d.romanick at 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.romanick at intel.com>
> Cc: Tapani Pälli <tapani.palli at intel.com>
> Cc: Matt Turner <mattst88 at gmail.com>
> Cc: mesa-stable at lists.freedesktop.org
> Fixes: ee63933a7 ("nir: Distribute binary operations with constants into bcsel")
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105704
> ---
> 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();
>
More information about the mesa-stable
mailing list