[Mesa-dev] [PATCH 2/2] st/mesa: set MaxUnrollIterations = 64

[Ian Romanick]

On 03/21/2012 09:03 AM, Jose Fonseca wrote:
>
>
> ----- Original Message -----
>> On 03/21/2012 05:42 AM, Jose Fonseca wrote:
>>> The increase sounds good to me.
>>>
>>> But 64 seems small. SM3 allows loops up to 255 iterations, and
>>> Microsoft HLSL compiler will unroll loops that big.
>>
>> I'm happy to raise the limit higher.  If there's no objections let's
>> go with 255.
>>
>>
>>> Although I wonder why we don't simply unroll everything until we
>>> hit PIPE_SHADER_CAP_MAX_INSTRUCTIONS too, when
>>> options->EmitNoLoops is false.
>>
>> Luca Barbieri wrote this code in commit
>> b37459388b22fb6a831d45bc08e51cdb7b9c610c.
>>
>> I'd be happy to set the MaxUnrollIterations unconditionally, without
>> regard to EmitNoLoops.  I was just trying to make the least invasive
>> change to fix the problem at hand (and I think I'd still prefer to do
>> that for the time being).
>>
>> We have to be careful with # of instructions vs. # of iterations.
>> Most loops have at least two instructions so
>> PIPE_SHADER_CAP_MAX_INSTRUCTIONS/2 would seem to be a better choice.
>> Though even that's probably unneccesarily large.
>>
>> I guess the key question is do we want smaller shaders with loops or
>> larger shaders without loops?
>
> I think I got this wrong. Please forget what I said.
>
>
> I thought that this path was only taken when the hardware can _not_ handle any loop (as  svga), but it's actually the other way around.
>
>
> Although svga advertises get_shader_param(PIPE_SHADER_FRAGMENT, PIPE_SHADER_CAP_INDIRECT_INPUT_ADDR) == 0, what it is trying to say is "please emit no loops", yet options->EmitNoLoops is False.
>
> The right to do here is get options->EmitNoLoops to be true for svga. Either make
>
>    options->EmitNoLoops = !PIPE_SHADER_CAP_MAX_CONTROL_FLOW_DEPTH || ! PIPE_SHADER_CAP_INDIRECT_INPUT_ADDR
>
> or add a new PIPE_SHADER_CAP_LOOPS or something.

Oh... so the problem is that when the loops aren't unrolled, it can't 
handle the array accesses in the loops?  So, things like

	float sum = 0.;
	for (int i = 0; i < 99; i++) {
		sum += my_array[i];
	}

end up really hurting.

Making EmitNoLoops depend on the existence of an address register is 
probably fair.  It may be better in the long run to add an additional 
control to the loop unroller.  It should be able to detect indirect 
addressing in the bodies of loops and force the loop to be unrolled 
regardless of the iteration count.