On 11/20/2017 03:32 PM, Matt Turner wrote: > On Mon, Nov 20, 2017 at 2:50 PM, Ian Romanick <[email protected]> wrote: >> On 11/20/2017 02:33 PM, Matt Turner wrote: >>> MADs don't take immediate sources, but we allow them in the IR since it >>> simplifies a lot of things. I neglected to consider that case. >>> >>> Fixes: 4009a9ead490 ("i965/fs: Allow saturate propagation to propagate >>> negations into MADs.") >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=103616 >>> Reported-and-Tested-by: Ruslan Kabatsayev <[email protected]> >>> --- >>> src/intel/compiler/brw_fs_saturate_propagation.cpp | 10 ++++++++-- >>> 1 file changed, 8 insertions(+), 2 deletions(-) >>> >>> diff --git a/src/intel/compiler/brw_fs_saturate_propagation.cpp >>> b/src/intel/compiler/brw_fs_saturate_propagation.cpp >>> index 1c97a507d8..d6cfa79a61 100644 >>> --- a/src/intel/compiler/brw_fs_saturate_propagation.cpp >>> +++ b/src/intel/compiler/brw_fs_saturate_propagation.cpp >>> @@ -88,8 +88,14 @@ opt_saturate_propagation_local(fs_visitor *v, bblock_t >>> *block) >>> scan_inst->src[0].negate = >>> !scan_inst->src[0].negate; >>> inst->src[0].negate = false; >>> } else if (scan_inst->opcode == BRW_OPCODE_MAD) { >>> - scan_inst->src[0].negate = >>> !scan_inst->src[0].negate; >>> - scan_inst->src[1].negate = >>> !scan_inst->src[1].negate; >>> + for (int i = 0; i < 2; i++) { >>> + if (scan_inst->src[i].file == IMM) { >>> + brw_negate_immediate(scan_inst->src[i].type, >>> + >>> &scan_inst->src[i].as_brw_reg()); >>> + } else { >>> + scan_inst->src[i].negate = >>> !scan_inst->src[i].negate; >>> + } >>> + } >> >> Is this going to affect the number of generated instructions if there >> are multiple MADs using the same immediate value for a multiply source? >> Would it be better to find a multiply source that isn't an immediate? > > Interesting question. I think the answer is no, since > brw_fs_combine_constants.cpp builds its list of constants by ignoring > their signs. Since source negation is free, it just loads positive > values into registers and negates them with a source modifier if > needed. > > I ran shader-db to confirm -- no changes on SKL.
In that case, this patch is also Reviewed-by: Ian Romanick <[email protected]> _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
