On Wed, Mar 30, 2011 at 10:09:58AM -0700, Ian Romanick wrote: > -----BEGIN PGP SIGNED MESSAGE----- > Hash: SHA1 > > On 03/29/2011 08:27 PM, Tom Stellard wrote: > > > > The reason there are reads of undefined values is because of all the > > conditional assignments generated by the IF to conditional assignment > > lowering pass. ir_to_mesa transforms conditional assignments to > > CMP DST, COND SRC0 DST, with the assumption that if the condition fails > > assigning DST to itself will be a noop. This is normally a safe assumption > > to make since all bug-free programs should initialize a value before using > > it in a conditional assignment, if the value is going to be used later > > in the program. > > > > However, the conditional assignments that are generated by the IF > > lowering pass don't follow this pattern and the DST register is usually > > uninitialized when the instruction is executed, leading to a read from > > an undefined value. > > Ah, that makes sense. It would probably be good to replace the examples > in the current header comment with one of these cases. > > > It seems like the real problem is that there is no good way to translate > > a GLSL IR conditional assignment to a MESA IR instruction. From what > > I can tell, the semantics of conditional assignment are: > > if (cond) > > assign value > > else > > do nothing; > > Correct. > > > and the closest equivalent Mesa IR instruction (CMP) is: > > if (cond) > > assign value0 > > else > > assign value1 > > > > Since this is only an issue on architectures that don't support flow > > control, I should modify my patch so the 'reads without writes' pass > > only runs on architectures that don't support flow control. I'm also > > interested in hearing alternate solutions if anyone has other ideas, > > because I would really like to get this fixed. > > It seems like there are two possible cases, and this doesn't really fix > either of them. > > Case 1: > > if (cond) > x = y; > > This gets lowered to a conditional move to x. If there is no 'else', > why do a conditional move at all? In this case, replace the CMP (which > may be lowered to as many as 3 instructions in some vertex shaders) with > a MOV. > > If x is used when cond is false before another assignment to x, the > result is undefined. Reading y qualifies as much as reading 0s. Right? :) > > Case 2: > > if (cond) > x = y; > else > x = z; > > In this case we end up with two CMP instructions. The first of the CMP > instructions will be potentially read from an uninitialized source. > > CMP x, cond, y, x; > CMP x, cond, x, z; > > In this case, the two CMP instructions should be fused into a single CMP. > > CMP x, cond, y, z; > > Now that I think about it, the way the copy propagation works, > converting the first CMP to a MOV will make this happen automatically. > > CMP x, cond, y, x; > CMP x, cond, x, z; > > becomes > > MOV x, y; > CMP x, cond, x, z; > > becomes > > MOV x, y; > CMP x, cond, y, z; > > becomes > > CMP x, cond, y, z;
Ok, this makes sense. It would be pretty easy to modify my original patch to do this, but maybe it's better change the way ir_to_mesa.cpp converts conditional assignments to Mesa IR. Do you have any suggestions for how to determine if the left hand side of an ir_assignment has already been written to? -Tom _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
