[Mesa-dev] [PATCH 10/14] glsl_to_tgsi: remove subroutine support

[Marek Olšák]

On Mon, Oct 17, 2016 at 4:10 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
> On Mon, Oct 17, 2016 at 9:59 AM, Marek Olšák <maraeo at gmail.com> wrote:
>> On Mon, Oct 17, 2016 at 3:48 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>> On Mon, Oct 17, 2016 at 9:46 AM, Marek Olšák <maraeo at gmail.com> wrote:
>>>> On Mon, Oct 17, 2016 at 3:44 PM, Ilia Mirkin <imirkin at alum.mit.edu> wrote:
>>>>> nouveau supports PIPE_SHADER_CAP_SUBROUTINES and properly details with
>>>>> RET opcodes. The alternative is that the st lowers the whole thing
>>>>> into a loop which adds IMHO unnecessary complexity to the resulting
>>>>> code. Any reason not to leave that in place?
>>>>
>>>> It had already been disabled and probably broken too. If you have no
>>>> interest in fixing and using it within a reasonable time frame, there
>>>> is no reason for the code to be there.
>>>
>>> What's broken about it? To the best of my knowledge, it's working
>>> fine. I'm specifically talking about RET (from the "MAIN" function),
>>> not subroutines in general.
>>
>> OK. I thought you were talking about subroutines.
>>
>> The RET opcode support can stay there if somebody wants it very much.
>>
>> However, have you seen any apps using "return" in "main"?
>
> Yes. Among other things, a lot of compute shaders do if (foo) return;
> somewhere at the top. See this example from F1 2015:
> http://hastebin.com/vacevigoyi.go - I've seen it in the middle too
> though.
>
>> Is there any serious performance concern with lowering "return" to a
>> conditional branch, such that it's beneficial for drivers to implement
>> RET?
>
> Depends on the driver. Nouveau's control flow analysis isn't extremely
> sophisticated to undo the lowering that the GLSL compiler does. Which
> means extra instructions, and extra RA complexity since the whole main
> function ends up inside a loop.
>
>> If so, shouldn't it be fixed in the GLSL compiler instead?
>
> Well, from what I recall, the GLSL compiler's lowering goes something like
>
> void main() {
>   if (foo) return;
>   ...
> }
>
> to
>
> void main() {
>   while (true) {
>      if (foo)
>         break;
>      ...
>      break;
>   }
> }
>
> With additional complexity if the return is inside a for/while loop itself.
>
>> Is it worth having a rarely-used codepath in glsl_to_tgsi that most
>> drivers skip anyway?
>
> It's 2 lines of code... one to stop the GLSL compiler from doing the
> lowering, and another to emit the RET opcode when you see the
> ir_op_return or whatever it's called. IMHO not such a great burden.
>
> All the other subroutines stuff can go as far as I'm concerned, I
> doubt anyone will care to implement glsl subroutines that way.
>
>   -ilia

OK, sounds good.

Marek