[Mesa-dev] [PATCH 2/3] glsl: add continue/break/return unification/elimination pass

[Ian Romanick]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Luca Barbieri wrote:
> This is a new pass that supersedes ir_if_return and "lowers" jumps
> to if/else structures.

One of the things we had done intention was keep individual passes
simple, even if that means doing more total work.  The reason is that it
makes it easier to add, remove, and modify individual passes.  Given a
quick glance at your patch, I'm not sure that's particularly applicable
here.

> Currently it causes no regressions on softpipe and nv40, but I'm not sure
> whether the piglit glsl tests are thorough enough, so consider this
> experimental.

I fully believe that our piglit tests provide woefully too little
coverage.  Honestly, that's one of the reasons it took so long for me to
merge the loop unrolling branch. :(

> It can be asked to:
> 1. Pull jumps out of ifs where possible
> 2. Remove all "continue"s, replacing them with an "execute flag"

This essentially wraps the rest of the code in the loop with
if-statements, right?  I had been thinking about adding a pass to do
this as well.

> 3. Replace all "break" with a single conditional one at the end of the loop

Hmm... I keep forgetting about conditional breaks.  We don't have much
support for them throughout the compiler.  We should probably fix that.
 Generating loop controls as conditional breaks (instead of 'if (cond)
break;') would be a good first step.  A pass that lowers other
if-statements that contain breaks to conditional breaks would be a good
second step.

> 4. Replace all "return"s with a single return at the end of the function,
>    for the main function and/or other functions
> 
> This gives several great benefits:
> 1. All functions can be inlined after this pass
> 2. nv40 and other pre-DX10 chips without "continue" can be supported
> 3. nv30 and other pre-DX10 chips with no control flow at all are better supported
> 
> Note that for full effect we should also teach the unroller to unroll
> loops with a fixed maximum number of iterations but with the canonical
> conditional "break" that this pass will insert if asked to.

Could you elaborate on this a bit?

> Continues are lowered by adding a per-loop "execute flag", initialized to
> TRUE, that when cleared inhibits all execution until the end of the loop.
> 
> Breaks are lowered to continues, plus setting a "break flag" that is checked
> at the end of the loop, and trigger the unique "break".
> 
> Returns are lowered to breaks/continues, plus adding a "return flag" that
> causes loops to break again out of their enclosing loops until all the
> loops are exited: then the "execute flag" logic will ignore everything
> until the end of the function.
> 
> Note that "continue" and "return" can also be implemented by adding
> a dummy loop and using break.
> However, this is bad for hardware with limited nesting depth, and
> prevents further optimization, and thus is not currently performed.

On which hardware will this make a difference?  It seems that any
hardware that has loops also has returns.  Or is this another case where
VS and FS hardware differs?

I haven't fully reviewed the code yet, I'll get to that tomorrow.  I do
have a couple code style comments.  About a month into things, we
started explicitly using 'this' in all methods.  For C programmers,
we've found that this makes things a lot more clear.  We've also been
trying to avoid initialization lists for the same reason.  I don't find
the argument in the C++ FAQ
(http://www.parashift.com/c++-faq-lite/ctors.html#faq-10.6) very compelling.

We've also been using FINISHME instead of TODO in comments.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkyFy44ACgkQX1gOwKyEAw+qCwCfUoS0T/v7DwwGPathJ0ovDg0Y
m0AAn0nCklbOZG11w81JWfzhSh0NZaq2
=rmRn
-----END PGP SIGNATURE-----