diff --git a/.github/workflows/source-regressions.yml b/.github/workflows/source-regressions.yml index 360bda10..d943751f 100644 --- a/.github/workflows/source-regressions.yml +++ b/.github/workflows/source-regressions.yml @@ -44,6 +44,7 @@ on: - tests/test_shader_census_cmd.py - gpu/vk/vk_shader.cpp - tests/test_same_draw.py + - tests/test_pass_hazard.py - nx2/version.py - nx2/test_version.py - host/nn_audio.c @@ -114,6 +115,7 @@ on: - tests/test_shader_census_cmd.py - gpu/vk/vk_shader.cpp - tests/test_same_draw.py + - tests/test_pass_hazard.py - nx2/version.py - nx2/test_version.py - host/nn_audio.c @@ -218,6 +220,9 @@ jobs: - name: Check a repeated uniform word is not read again if: runner.os == 'Linux' run: python -I -B tests/test_ub_memo.py + - name: Check a pass hazard skips textures that are not targets + if: runner.os == 'Linux' + run: python -I -B tests/test_pass_hazard.py - name: Check a repeated read bind is not looked up again if: runner.os == 'Linux' run: python -I -B tests/test_bind_memo.py diff --git a/gpu/vk/vk_draw.cpp b/gpu/vk/vk_draw.cpp index 0aeaed65..7a867542 100644 --- a/gpu/vk/vk_draw.cpp +++ b/gpu/vk/vk_draw.cpp @@ -2114,6 +2114,7 @@ struct Pass { uint64_t first_prog = 0; VkFormat format0 = VK_FORMAT_UNDEFINED; std::vector sampled; // (rendered textures its draws sampled: a target among them is a hazard) + uint16_t sampled_targets = 0; // (colour i bit i, depth bit 8: targets earlier draws sampled) uint16_t written = 0; // (its targets written so far: colour i bit i, depth bit 8) }; thread_local Pass pass; @@ -2247,7 +2248,7 @@ void begin_pass(VkCommandBuffer cmd, const Targets& at) { pass.draws = 0, pass.format0 = at.n ? at.color_format[0] : at.depth_format; pass.wm_or = 0, pass.wm_n = 0; pass.clears = 0, pass.first_prog = 0; - pass.sampled.clear(), pass.written = 0; + pass.sampled.clear(), pass.sampled_targets = 0, pass.written = 0; vkCmdBeginRendering(cmd, &ri); pass.open = true; st_passes++; @@ -2617,6 +2618,9 @@ void clear_after_draws(VkCommandBuffer cmd) { } int64_t st_pass_hazards; +bool writes_sampled_target(uint16_t writes, uint16_t sampled_targets) { + return (writes & sampled_targets) != 0; +} /// A draw that samples a target this pass already wrote, or writes a target an earlier draw of this pass sampled: in /// a pass of its own, after a full barrier (begin_pass makes it), as for the clears above: in one pass the GPU orders /// the targets' writes with each other only, not with the draws' texture reads of them. (A draw that samples a target @@ -2641,11 +2645,8 @@ uint16_t draw_hazards(VkCommandBuffer cmd, const Program& p, const NvnState& s, const int k = slot_of(t); if (k >= 0 && (pass.written >> k & 1) && !(writes >> k & 1)) { kind = "samples a target the pass wrote"; break; } } - if (!kind) - for (const Texture* t : pass.sampled) { - const int k = slot_of(t); - if (k >= 0 && (writes >> k & 1)) { kind = "writes a target an earlier draw sampled"; break; } - } + if (!kind && writes_sampled_target(writes, pass.sampled_targets)) + kind = "writes a target an earlier draw sampled"; if (!kind) return writes; static int logged; if (logged++ < 8) @@ -4104,8 +4105,15 @@ void draw(VkCommandBuffer cmd, NvnState& s, const DrawArgs& d) { probe_draw(s, p, pass.draws); // (hookshot: RX_VK_PROBE) pass.prog = p.key; if (!pass.first_prog) pass.first_prog = p.key; - for (const Texture* t : gfx_sampled) - if (std::find(pass.sampled.begin(), pass.sampled.end(), t) == pass.sampled.end()) pass.sampled.push_back(t); + for (const Texture* t : gfx_sampled) { + if (std::find(pass.sampled.begin(), pass.sampled.end(), t) != pass.sampled.end()) continue; + pass.sampled.push_back(t); + int slot = -1; + for (int i = 0; i < at.n; i++) + if (at.color[i] == t) { slot = i; break; } + if (slot < 0 && at.depth == t) slot = 8; + if (slot >= 0) pass.sampled_targets |= (uint16_t)(1u << slot); + } pass.written |= writes; if (pl != last_pipeline) vkCmdBindPipeline(cmd, VK_PIPELINE_BIND_POINT_GRAPHICS, pl), last_pipeline = pl; if (!same_desc) { diff --git a/tests/test_pass_hazard.py b/tests/test_pass_hazard.py new file mode 100644 index 00000000..2d223489 --- /dev/null +++ b/tests/test_pass_hazard.py @@ -0,0 +1,69 @@ +"""A pass hazard does not walk every texture the pass has sampled. + +Earlier draws record the targets they sampled as bits. A later draw that writes one +of those targets is the hazard. python -I -B tests/test_pass_hazard.py [--cxx g++] +""" +import argparse +import subprocess +import tempfile +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] + + +def block(text, marker): + start = text.index(marker) + end = text.index("{", start) + 1 + depth = 1 + while depth: + depth += (text[end] == "{") - (text[end] == "}") + end += 1 + return text[start:end] + + +def main(): + p = argparse.ArgumentParser(description=__doc__) + p.add_argument("--cxx", default="g++") + a = p.parse_args() + source = (ROOT / "gpu" / "vk" / "vk_draw.cpp").read_text(encoding="utf-8") + haz = block(source, "uint16_t draw_hazards(") + if "for (const Texture* t : pass.sampled)" in haz: + raise SystemExit("a pass hazard walks every texture sampled so far") + if "writes_sampled_target(" not in haz: + raise SystemExit("a pass hazard ignores the targets earlier draws sampled") + if haz.index("gfx_sampled") > haz.index("writes_sampled_target("): + raise SystemExit("a draw's own samples are treated as an earlier draw's") + begin = block(source, "void begin_pass(") + clear = begin.index("pass.sampled.clear()") + if "sampled_targets = 0" not in begin[clear:clear + 80]: + raise SystemExit("a new pass keeps the targets earlier draws sampled") + ins = source.index("pass.sampled.push_back(t);") + wrote = source.index("pass.written |= writes;", ins) + if "sampled_targets" not in source[ins:wrote]: + raise SystemExit("a sampled target is invisible to the next draw") + fn = block(source, "bool writes_sampled_target(") + fixture = f'''#include +#include +#include +#define CHECK(x) do {{ if (!(x)) {{ std::fprintf(stderr, "FAIL:%d %s\\n", __LINE__, #x); std::exit(1); }} }} while (0) +{fn} +int main() {{ + CHECK(writes_sampled_target(1u, 1u)); + CHECK(writes_sampled_target(0x100u, 0x100u)); + CHECK(writes_sampled_target(0x5u, 0x4u)); + CHECK(!writes_sampled_target(1u, 2u)); + CHECK(!writes_sampled_target(0u, 0x1FFu)); + CHECK(!writes_sampled_target(0x1FFu, 0u)); + std::printf("PASS: a draw hazards only the targets earlier draws sampled\\n"); +}} +''' + with tempfile.TemporaryDirectory(prefix="pass-hazard-") as tmp: + cpp = Path(tmp) / "fixture.cpp" + exe = Path(tmp) / "fixture" + cpp.write_text(fixture, encoding="utf-8") + subprocess.run([a.cxx, "-std=c++20", "-Wall", "-Wextra", "-Werror", str(cpp), "-o", str(exe)], check=True) + subprocess.run([str(exe)], check=True) + + +if __name__ == "__main__": + main()