From 222b47c6c569375ad90f6192d8a993cd081fcd69 Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Wed, 22 Jan 2020 16:23:14 -0800 Subject: intel/fs: Don't count integer instructions as being possibly coissue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integer instructions don't coissue. Before e64be391dd0 ("intel/compiler: generalize the combine constants pass"), this pass only looked at float sources. There's no shader-db data in that commit, so I collected some. The results are not good: Haswell total instructions in shared programs: 11898805 -> 11908127 (0.08%) instructions in affected programs: 1218680 -> 1228002 (0.76%) helped: 2 HURT: 5171 helped stats (abs) min: 12 max: 111 x̄: 61.50 x̃: 61 helped stats (rel) min: 1.59% max: 9.20% x̄: 5.40% x̃: 5.40% HURT stats (abs) min: 1 max: 311 x̄: 1.83 x̃: 1 HURT stats (rel) min: 0.02% max: 9.91% x̄: 1.05% x̃: 0.70% 95% mean confidence interval for instructions value: 1.55 2.05 95% mean confidence interval for instructions %-change: 1.02% 1.08% Instructions are HURT. total cycles in shared programs: 221664974 -> 221404750 (-0.12%) cycles in affected programs: 120012620 -> 119752396 (-0.22%) helped: 3464 HURT: 3159 helped stats (abs) min: 1 max: 428160 x̄: 314.55 x̃: 16 helped stats (rel) min: <.01% max: 57.33% x̄: 3.40% x̃: 1.28% HURT stats (abs) min: 1 max: 87846 x̄: 262.54 x̃: 14 HURT stats (rel) min: <.01% max: 85.57% x̄: 3.01% x̃: 0.77% 95% mean confidence interval for cycles value: -224.23 145.65 95% mean confidence interval for cycles %-change: -0.50% -0.19% Inconclusive result (value mean confidence interval includes 0). total spills in shared programs: 9804 -> 10047 (2.48%) spills in affected programs: 6869 -> 7112 (3.54%) helped: 2 HURT: 41 total fills in shared programs: 19863 -> 20319 (2.30%) fills in affected programs: 17428 -> 17884 (2.62%) helped: 2 HURT: 41 LOST: 20 GAINED: 13 This also prevents regressions in "intel/fs: Promote integer constants after lowering integer multiplication" (note: that patch will probably not be committed). When the passes are reorderd, code like mul(8) acc0<1>D g9<8,8,1>D -2078209981D { align1 1Q }; gets turned into mov(1) g23<1>D 2078209981D { align1 WE_all 1N }; ... mul(8) acc0<1>D g13<8,8,1>D -g23<0,1,0>D { align1 1Q compacted }; It's not 100% clear why, but these produce different results. Note that -2078209981 & 0x0ffff = 0x0843, and -(2078209981 & 0x0ffff) = 0xffff0843. It seems like the upper 16-bits of the negation should be ignored. Fixes: e64be391dd0 ("intel/compiler: generalize the combine constants pass") Cc: Iago Toral Quiroga Suggested-by: Matt Turner Reviewed-by: Matt Turner The shaders with spills or fills hurt are the usual suspects. A couple compute shaders in Dirt Showdown and a compute shader in Bioshock Infinite. On Haswell, a compute shader (that appears twice in shader-db) from Aztec Ruins was also hurt for spill and fills. Haswell total instructions in shared programs: 11573934 -> 11568335 (-0.05%) instructions in affected programs: 828623 -> 823024 (-0.68%) helped: 2825 HURT: 6 helped stats (abs) min: 1 max: 134 x̄: 2.16 x̃: 1 helped stats (rel) min: 0.02% max: 9.05% x̄: 0.84% x̃: 0.61% HURT stats (abs) min: 1 max: 216 x̄: 81.83 x̃: 56 HURT stats (rel) min: 0.16% max: 8.65% x̄: 4.21% x̃: 4.68% 95% mean confidence interval for instructions value: -2.31 -1.64 95% mean confidence interval for instructions %-change: -0.85% -0.80% Instructions are helped. total cycles in shared programs: 187573593 -> 187004633 (-0.30%) cycles in affected programs: 82816107 -> 82247147 (-0.69%) helped: 2186 HURT: 1741 helped stats (abs) min: 1 max: 35230 x̄: 326.96 x̃: 16 helped stats (rel) min: <.01% max: 46.11% x̄: 3.11% x̃: 0.90% HURT stats (abs) min: 1 max: 6138 x̄: 83.73 x̃: 16 HURT stats (rel) min: <.01% max: 104.11% x̄: 2.73% x̃: 0.75% 95% mean confidence interval for cycles value: -197.13 -92.64 95% mean confidence interval for cycles %-change: -0.72% -0.33% Cycles are helped. total spills in shared programs: 7870 -> 7743 (-1.61%) spills in affected programs: 2260 -> 2133 (-5.62%) helped: 31 HURT: 5 total fills in shared programs: 6320 -> 6263 (-0.90%) fills in affected programs: 3547 -> 3490 (-1.61%) helped: 31 HURT: 6 LOST: 9 GAINED: 9 Ivybridge total instructions in shared programs: 11863372 -> 11859793 (-0.03%) instructions in affected programs: 757183 -> 753604 (-0.47%) helped: 2236 HURT: 3 helped stats (abs) min: 1 max: 81 x̄: 1.86 x̃: 1 helped stats (rel) min: 0.03% max: 5.26% x̄: 0.74% x̃: 0.48% HURT stats (abs) min: 11 max: 301 x̄: 192.33 x̃: 265 HURT stats (rel) min: 1.55% max: 10.51% x̄: 6.89% x̃: 8.62% 95% mean confidence interval for instructions value: -2.01 -1.18 95% mean confidence interval for instructions %-change: -0.77% -0.70% Instructions are helped. total cycles in shared programs: 178377378 -> 177946087 (-0.24%) cycles in affected programs: 76261390 -> 75830099 (-0.57%) helped: 1635 HURT: 1395 helped stats (abs) min: 1 max: 34796 x̄: 333.53 x̃: 16 helped stats (rel) min: <.01% max: 47.15% x̄: 2.82% x̃: 0.64% HURT stats (abs) min: 1 max: 4315 x̄: 81.74 x̃: 18 HURT stats (rel) min: <.01% max: 49.98% x̄: 1.99% x̃: 0.53% 95% mean confidence interval for cycles value: -197.06 -87.62 95% mean confidence interval for cycles %-change: -0.78% -0.43% Cycles are helped. total spills in shared programs: 4188 -> 4182 (-0.14%) spills in affected programs: 1557 -> 1551 (-0.39%) helped: 30 HURT: 3 total fills in shared programs: 5056 -> 5245 (3.74%) fills in affected programs: 2708 -> 2897 (6.98%) helped: 30 HURT: 3 LOST: 5 GAINED: 1 No shader-db changes on any other Intel platform. Tested-by: Marge Bot Part-of: (cherry picked from commit 59488cbbaca1268841fe5ba42d0a1202b33be23b) --- .pick_status.json | 2 +- src/intel/compiler/brw_fs_combine_constants.cpp | 9 ++++++++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 2922bfb5d6a..5541ef7886f 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -58,7 +58,7 @@ "description": "intel/fs: Don't count integer instructions as being possibly coissue", "nominated": true, "nomination_type": 1, - "resolution": 0, + "resolution": 1, "master_sha": null, "because_sha": "e64be391dd065b6a0eabee17ada038db7a28c112" }, diff --git a/src/intel/compiler/brw_fs_combine_constants.cpp b/src/intel/compiler/brw_fs_combine_constants.cpp index 2a28351beef..d10ac0219cf 100644 --- a/src/intel/compiler/brw_fs_combine_constants.cpp +++ b/src/intel/compiler/brw_fs_combine_constants.cpp @@ -56,7 +56,14 @@ could_coissue(const struct gen_device_info *devinfo, const fs_inst *inst) case BRW_OPCODE_CMP: case BRW_OPCODE_ADD: case BRW_OPCODE_MUL: - return true; + /* Only float instructions can coissue. We don't have a great + * understanding of whether or not something like float(int(a) + int(b)) + * would be considered float (based on the destination type) or integer + * (based on the source types), so we take the conservative choice of + * only promoting when both destination and source are float. + */ + return inst->dst.type == BRW_REGISTER_TYPE_F && + inst->src[0].type == BRW_REGISTER_TYPE_F; default: return false; } -- cgit v1.2.3