Iago Toral <ito...@igalia.com> writes:
> On Tue, 2015-10-13 at 15:17 +0300, Francisco Jerez wrote:
>> Iago Toral Quiroga <ito...@igalia.com> writes:
>>
>> > This fixes the following test:
>> >
>> > [require]
>> > GL >= 3.3
>> > GLSL >= 3.30
>> > GL_ARB_shader_storage_buffer_object
>> >
>> > [fragment shader]
>> > #version 330
>> > #extension GL_ARB_shader_storage_buffer_object: require
>> >
>> > buffer SSBO {
>> > mat4 sm4;
>> > };
>> >
>> >
>> > mat4 um4;
>> >
>> > void main() {
>> > sm4 *= um4;
>>
>> This is using the value of "um4", which is never assigned, so liveness
>> analysis will correctly extend its live interval up to the start of the
>> block.
>>
>> The other problem here seems to be that the liveness analysis pass
>> doesn't consider scalar writes (like the ones emitted by the
>> combine_constants optimization pass and by emit_uniformize()) to fully
>> define the contents of a register, so it will incorrectly extend up the
>> live interval of registers defined using scalar writes even if all
>> components ever used in the shader have been defined individually.
>> Incidentally the use-def-chains-based implementation of liveness
>> analysis I'm working on will fix this issue easily.
>
> Great, thanks Curro! Once you make your change public I can verify that
> they fix this too.
>
> BTW, even if your changes fix this I wonder if my patch is still valid:
> control-flow analysis should not add anything relevant to liveness
> analysis if we only have one block, right?
>Yeah, that should be okay. I'm not strongly opposed to this change as temporary hack-around for the meantime, but the comments seem somewhat misleading. Either way patches 1 and 3 (i.e. the indentation fixes) are: Reviewed-by: Francisco Jerez <curroje...@riseup.net> > Iago > >> > sm4[0][0] = 0.0; >> > } >> > >> > [test] >> > link success >> > >> > where our liveness analysis works really bad because the control-flow part >> > will expand register liveness to the end of only block we have (so every >> > register will be marked alive until the end of the program). This >> > artificially >> > increases register pressure to a point in which we run out of registers. >> > Of course, this is only a simple optimization for a trivial case, the >> > underlying problem still exists and would manifest when we have more than >> > one block, but it helps simple shaders such as the one in the example above >> > without any effort. I guess the real fix would involve re-thinking parts >> > of the >> > liveness analysis algorithm... >> > >> > Jordan, there is another thing that we can improve for this shader that is >> > specific to SSBOs: we emit the same ssbo load multiple times because we are >> > playing it safe just in case there are writes in between. I think we can >> > try to >> > do better and not re-issue the same load if we don't have ssbo stores to >> > the same address in between. I'll try to come up with a patch for this >> > (hopefully we can do this inside lower_ubo_reference). >> > >> > The actual fix goes into patches 2 (FS) and 4 (VS). Patches 1,3 are just >> > indentation fixes in the code around these. >> > >> > Iago Toral Quiroga (4): >> > i965/fs: Fix indentation in fs_live_variables::compute_start_end >> > i965/fs: skip control-flow aware liveness analysis if we only have 1 >> > block >> > i965/vec4: fix indentation in vec4_visitor::calculate_live_intervals >> > i965/vec4: skip control-flow aware liveness analysis if we only have 1 >> > block >> > >> > .../drivers/dri/i965/brw_fs_live_variables.cpp | 24 >> > ++++++++++++++-------- >> > .../drivers/dri/i965/brw_vec4_live_variables.cpp | 23 >> > +++++++++++++-------- >> > 2 files changed, 30 insertions(+), 17 deletions(-) >> > >> > -- >> > 1.9.1 >> > >> > _______________________________________________ >> > mesa-dev mailing list >> > mesa-dev@lists.freedesktop.org >> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev
signature.asc
Description: PGP signature
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
