From 2ea6b340ac29a7710ca748fa2fff4fdd633f2f9f Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Fri, 24 Jan 2025 10:43:28 -0800 Subject: [PATCH] brw/copy: Fix handling of offset in extract_imm The offset is measured in bytes. Some of the code here acted as though it were measured in src.type units. Also modify the assertion to check that all extracted bits come from data in the immediate value. Fixes: 580e1c592d90 ("intel/brw: Introduce a new SSA-based copy propagation pass") Fixes: da395e6985a ("intel/brw: Fix extract_imm for subregion reads of 64-bit immediates") Yes, I missed this error *twice* in code review. Reviewed-by: Kenneth Graunke Part-of: (cherry picked from commit ac4b93571cfba02bcbd589f2008f615ecf1813c4) --- .pick_status.json | 2 +- src/intel/compiler/brw_opt_copy_propagation.cpp | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 81ce62871f2..676321478b6 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -494,7 +494,7 @@ "description": "brw/copy: Fix handling of offset in extract_imm", "nominated": true, "nomination_type": 2, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": "580e1c592d90392a30185d8059499498748909fd", "notes": null diff --git a/src/intel/compiler/brw_opt_copy_propagation.cpp b/src/intel/compiler/brw_opt_copy_propagation.cpp index 94744b798a9..fe78cbc6924 100644 --- a/src/intel/compiler/brw_opt_copy_propagation.cpp +++ b/src/intel/compiler/brw_opt_copy_propagation.cpp @@ -1745,9 +1745,12 @@ extract_imm(brw_reg val, brw_reg_type type, unsigned offset) if (offset == 0 || bitsize == brw_type_size_bits(val.type)) return val; - assert(bitsize < brw_type_size_bits(val.type)); + /* The whole extracted value must come from bits that acutally exist in the + * original immediate value. + */ + assert((8 * offset) + bitsize <= brw_type_size_bits(val.type)); - val.u64 = (val.u64 >> (bitsize * offset)) & ((1ull << bitsize) - 1); + val.u64 = (val.u64 >> (8 * offset)) & ((1ull << bitsize) - 1); return val; }