hlsl: ps_1_* outputs.
Merge request reports
Activity
2581 size_t i; 2576 2582 2577 for (i = 0; i < 4; ++i) 2583 for (i = 0; i < allocator->count; ++i) 2578 2584 { 2579 if (allocator->regs[component_idx + i].last_read <= first_write) 2580 { 2581 writemask |= 1u << i; 2582 if (++count == reg_size) 2583 return writemask; 2584 } 2585 const struct allocation *allocation = &allocator->allocations[i]; 2586 2587 /* We do not overlap if first write == last read: 2588 * this is the case where we are allocating the result of that 2589 * expression, e.g. "add r0, r0, r1". */ I felt sort of the same confusion. Part of it is that "We do not overlap" can mean two things: "There is not overlap when fw == lr, so we can proceed" (the intended meaning) and "We (the programmers) do not want to create an overlap by accepting this case in which fw == lr, so we cannot proceed". I guess this might be a general problem with ergative verbs with an unclear pronoun (and I generally interpret "we" as the programmers in program comments).
"There is no overlap" feels much clearer to me.
The second instance of "we" ("we are allocating") is probably more clear, but maybe "we are" can be dropped there too without sacrificing grammaticality and without requiring the random reader to figure out who is "we".
I haven't fully and thoroughly reviewed this MR but it does seem to be generally fine. Note to the fellow reviewer: the new get_available_writemask() is a pretty different function from the old one (WRT how it's used mostly), it just happens to cover the same "idea" and thus share the same name.
added 31 commits
-
bf3c1862...b46df551 - 27 commits from branch
wine:master
- 3c5e9f08 - vkd3d-shader/hlsl: Rename struct liveness to struct register_allocator.
- 83e1b8f6 - vkd3d-shader/hlsl: Avoid leaking the allocator register map in allocate_const_registers().
- aa8111b4 - vkd3d-shader/hlsl: Rewrite the register allocator to allow allocating in multiple passes.
- 24cbee07 - vkd3d-shader/hlsl: Map the colour output for ps_1_* to r0.
Toggle commit list-
bf3c1862...b46df551 - 27 commits from branch
The MR is ok for me, except for the little remark about comment wording.
I just wanted to notice that the new algorithm has higher computational complexity than before, because
get_available_writemask()
used to be constant time and it's now linear in the number of register allocations. This already causes a measurable performance hit in a synthetic but still relatively simple shader as this:uniform float4x4 x; uniform float4x4 y; float4 main(float4 pos : sv_position) : sv_target { float4x4 a = mul(mul(y, x), mul(x, x)); float4x4 b = mul(mul(y, x), mul(y, x)); float4x4 c = mul(mul(y, y), mul(x, x)); float4x4 d = mul(mul(y, y), mul(y, x)); float4 ret = 0.0; ret += a[0] - b[0] * c[0] / d[0]; ret += a[1] - b[1] * c[1] / d[1]; ret += a[2] - b[2] * c[2] / d[2]; ret += a[3] - b[3] * c[3] / d[3]; return ret; }
Here I am leveraging
mul()
to create a lot of temporaries and summing everything to prevent DCE from optimizing too much. On my computer a shader runner that just compiles this (doesn't execute it) takes 0.1 seconds before this MR and 0.11 seconds after it.I don't claim any significance for my random microbenchmark experiment, so I don't think it's necessary to change the MR, but when and if we'll be harvesting for performances in the HLSL compiler let's remember to have a look here.
added 59 commits
-
24cbee07...7917a682 - 55 commits from branch
wine:master
- c57ac0b2 - vkd3d-shader/hlsl: Rename struct liveness to struct register_allocator.
- 71d8ff85 - vkd3d-shader/hlsl: Avoid leaking the allocator register map in allocate_const_registers().
- b2959739 - vkd3d-shader/hlsl: Rewrite the register allocator to allow allocating in multiple passes.
- 8b57a612 - vkd3d-shader/hlsl: Map the colour output for ps_1_* to r0.
Toggle commit list-
24cbee07...7917a682 - 55 commits from branch