On 02/22/2016 04:16 PM, Ian Romanick wrote: > On 02/22/2016 04:05 PM, Matt Turner wrote: >> On Mon, Feb 22, 2016 at 11:42 AM, Ian Romanick <[email protected]> wrote: >>> From: Ian Romanick <[email protected]> >>> >>> Previously loops like >>> >>> do { >>> // ... >>> } while (false); >>> >>> that did not have any other loop-branch instructions would not be >>> unrolled. This is commonly used to wrap multiline preprocessor macros. >>> >>> This produces IR like >>> >>> (loop ( >>> ... >>> break >>> )) >>> >>> Since limiting_terminator was NULL, the loop unroller would >>> throw up its hands and say, "I don't know how many iterations. How >>> can I unroll this?" >>> >>> We can detect this another way. If there is no limiting_terminator >>> and the only loop-branch is a break as the last IR, there's only one >>> iteration. >>> >>> On my very old checkout of shader-db, this removes a loop from Orbital >>> Explorer, but it does not otherwise affect the shader. The loop removed >>> is the one the compiler inserts surrounding the switch statement. >> >> Orbital Explorer has a dead while loop because of >> >> commit 73dd50acf6d244979c2a657906aa56d3ac60d550 >> Author: Tapani Pälli <[email protected]> >> Date: Wed Aug 6 09:46:54 2014 +0300 >> >> glsl: implement switch flow control using a loop >> >> I don't understand how this patch interacts with that nor why it >> doesn't break Orbital Explorer rendering (I checked). >> >> The Orbital Explorer shader *does* have other loop-branch >> instructions, so it seems like this patch shouldn't have affected it? > > I will dig into this more. There are a lot of break instructions in the > "before" IR, and they all disappear. There's something fishy going on...
I see what's happening. Each case in the switch-statement ends with a return instead of a break. Eventually the function gets inlined and the returns are replaced with breaks. However, this code causes the loop to be removed, so the returns aren't replaced with breaks. I spent probably an hour poking at this, and think the only way to hit it is to have a switch-statement in main that uses returns from the cases. For all other cases, the returns get lowered before loop analysis occurs. I tried to come up with a test case involving conditional and unconditional returns from a switch with and without falling through... I couldn't come up with one that this patch broke. Perhaps someone more clever can. It really seems like the way this changes the IR should cause something to malfunction. :( Opinions about the patch? > _______________________________________________ > mesa-dev mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/mesa-dev _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
