From 9909b84f5b53578f233f792d665d049099792129 Mon Sep 17 00:00:00 2001 From: Caio Oliveira Date: Fri, 8 Mar 2024 08:36:03 -0800 Subject: [PATCH] intel/brw: Use helper to create accumulator register This ensure the region triple is set correctly, in this case the desired region is a sequential like <8,8,1>. Without the helper the sequence we get is <0,1,0> -- which the generator currently partially adjusts when emitting code, but is not sufficient when doing validation earlier. The code generated code is slightly modified. From crucible test func.shader.subtractSaturate.uint in the fragment shader for SIMD8, the diff looks like ``` mov(8) acc0<1>UD g21<8,8,1>UD { align1 1Q $0.dst }; -add.sat(8) g22<1>UD -acc0<0,1,0>UD g16<8,8,1>UD { align1 1Q @1 $0.dst }; +add.sat(8) g22<1>UD -acc0<8,8,1>UD g16<8,8,1>UD { align1 1Q @1 $0.dst }; ``` Note that without the patch generator adjusted the hstride for acc0 used as destination (see brw_set_dest), but kept the src region as is. For the source, it is not clear to me why the <0,1,0> would work correctly here since it is a scalar, but using <8,8,1> it is correct. Fixes: 58907568ec5 ("intel/fs: Add SHADER_OPCODE_[IU]SUB_SAT pseudo-ops") Reviewed-by: Francisco Jerez Reviewed-by: Ian Romanick Part-of: (cherry picked from commit db8022dc4d30cc1dc2903f1a8be0b2354d406357) --- .pick_status.json | 2 +- src/intel/compiler/brw_fs.cpp | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 67e1a8ef2f6..94c77ed5562 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -484,7 +484,7 @@ "description": "intel/brw: Use helper to create accumulator register", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "58907568ec526df87fa87177441743fa0d1d0a66", "notes": null diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp index b4b8f94992d..5008f64f7b0 100644 --- a/src/intel/compiler/brw_fs.cpp +++ b/src/intel/compiler/brw_fs.cpp @@ -4473,7 +4473,8 @@ fs_visitor::lower_sub_sat() */ if (inst->exec_size == 8 && inst->src[0].type != BRW_REGISTER_TYPE_Q && inst->src[0].type != BRW_REGISTER_TYPE_UQ) { - fs_reg acc(ARF, BRW_ARF_ACCUMULATOR, inst->src[1].type); + fs_reg acc = retype(brw_acc_reg(inst->exec_size), + inst->src[1].type); ibld.MOV(acc, inst->src[1]); fs_inst *add = ibld.ADD(inst->dst, acc, inst->src[0]);