summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorIan Romanick <ian.d.romanick@intel.com>2020-02-11 12:00:00 -0800
committerDylan Baker <dylan@pnwbakers.com>2020-02-20 13:36:39 -0800
commitea1899f8625e74b6f42c2601f650465dbafad223 (patch)
treef2f7eb44e539ba71036573825dd31c3b187c82b5
parent2a807e98b235bc7198c08f6b7fe8c2b1cc2d834b (diff)
intel/fs: Correctly handle multiply of fsign with a source modifier
The other source of the multiply will be interpreted as a uint32_t in an XOR instruction. Any source modifiers with either not be interpreted at all or will be misinterpreted due to the differing types. If the other operand of the multiplication has a source modifier, just emit an extra move to resolve the source modifiers. The negation source modifier problem is difficult to reproduce due to an algebraic optimization that changes (-a*b) to -(a*b). However, changes in MR !1359 push the negations back down. On Gen7+ it might be possible to do slightly better for an abs() source modifier by using BFI2 as a glorified copysign(). On Gen8+ it might be possible to do slightly better for a neg() source modifier by emitting (~a ^ b). There were no shader-db changes on any Intel platform, so I think we can deal with that problem when it arises. See also piglit!224. Fixes: 06d2c116415 ("intel/fs: Add a scale factor to emit_fsign") Reviewed-by: Matt Turner <mattst88@gmail.com> Tested-by: Marge Bot <https://gitlab.freedesktop.org/mesa/mesa/merge_requests/3780> Part-of: <https://gitlab.freedesktop.org/mesa/mesa/merge_requests/3780> (cherry picked from commit 273b8cd1ca286e2f43b4a464a391fdcaac49f077)
-rw-r--r--.pick_status.json2
-rw-r--r--src/intel/compiler/brw_fs_nir.cpp10
2 files changed, 11 insertions, 1 deletions
diff --git a/.pick_status.json b/.pick_status.json
index 33a365be759..2787b9c9c01 100644
--- a/.pick_status.json
+++ b/.pick_status.json
@@ -76,7 +76,7 @@
"description": "intel/fs: Correctly handle multiply of fsign with a source modifier",
"nominated": true,
"nomination_type": 1,
- "resolution": 0,
+ "resolution": 1,
"master_sha": null,
"because_sha": "06d2c116415c0ab163a57ed7f2522342ed43e4d4"
},
diff --git a/src/intel/compiler/brw_fs_nir.cpp b/src/intel/compiler/brw_fs_nir.cpp
index 2d6b248d5fe..8b5d819dda8 100644
--- a/src/intel/compiler/brw_fs_nir.cpp
+++ b/src/intel/compiler/brw_fs_nir.cpp
@@ -880,6 +880,16 @@ fs_visitor::emit_fsign(const fs_builder &bld, const nir_alu_instr *instr,
}
op[0] = offset(op[0], bld, fsign_instr->src[0].swizzle[channel]);
+
+ /* Resolve any source modifiers. We could do slightly better on Gen8+
+ * if the only source modifier is negation, but *shrug*.
+ */
+ if (op[1].negate || op[1].abs) {
+ fs_reg tmp = bld.vgrf(op[1].type);
+
+ bld.MOV(tmp, op[1]);
+ op[1] = tmp;
+ }
} else {
assert(!instr->dest.saturate);
}