On 7 December 2011 13:03, Eric Anholt <<a href="mailto:eric@anholt.net">eric@anholt.net</a>> wrote: <div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;"> <div class="im">On Wed, 7 Dec 2011 11:09:11 -0800, Paul Berry <<a href="mailto:stereotype441@gmail.com">stereotype441@gmail.com</a>> wrote: > - assert(!p->single_program_flow); > + /* In principle, we shouldn't be patching IF and ELSE instructions in > + * single program flow mode when gen < 6, because in single program flow > + * mode on those platforms, we convert flow control instructions to > + * conditional ADDs that operate on IP (see brw_ENDIF). > + * > + * However, on Gen6, writing to IP doesn't work in single program flow mode > + * (see the SandyBridge PRM, Volume 4 part 2, p79: "When SPF is ON, IP may > + * not be updated by non-flow control instructions."). And on later > + * platforms, there is no significant benefit to converting control flow > + * instructions to conditional ADDs. So we do patch IF and ELSE > + * instructions in single program flow mode on those platforms. > + */ > + if (intel->gen < 6) > + assert(!p->single_program_flow); </div>I stumbled over reading this comment the first couple of times. I think s/In principle,// would have made it clear for me -- I read the "However on Gen6" paragraph as trying to explain an exception to "in principle, ... when gen < 6", when it's really true that we don't patch if/else in SPF for gen < 6. > </blockquote></div> Whoops, you're right. I'll fix this before pushing.