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

[Ilia Mirkin]

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