On Nov 9, 2015 7:24 AM, "Connor Abbott" <<a href="mailto:cwabbott0@gmail.com">cwabbott0@gmail.com</a>> wrote: > > On Mon, Nov 9, 2015 at 6:55 AM, Iago Toral <<a href="mailto:itoral@igalia.com">itoral@igalia.com</a>> wrote: > > Hi, > > > > Currently, NIR defines vecN operations as unsigned (integer). The fp64 > > patches from Connor change this to float (I guess because we need to > > know the case where we are packing vectors of 64-bit floats). However, > > this makes it so that nir_lower_source_to_mods turns this: > > > > vec1 ssa_2 = fmov -ssa_1.y > > vec3 ssa_3 = vec3 ssa_1, ssa_2, ssa_0 > > > > into: > > > > vec3 ssa_2 = vec3 ssa_1, -ssa_1.y, ssa_0 > > > > This only happens because the vec3 operation is defined as a float > > operation now, otherwise it would not try to do this. It is not clear to > > me if this is by design, I mean, have this kind of things only kick-in > > for float/int and define vecN operations as unsigned to avoid this for > > them. > > > > The problem comes later when we call nir_lower_vec_to_movs in the i965 > > vec4 backend. That pass generates a separate MOV for each component in > > the vector, but to do that properly when a negate is involved it needs > > to know if this is a float or an integer operand, which it does not > > know at this point. The current code always emits an imov, which won't > > work if the operand is a float. > > > > I can think of two solutions for this: > > > > 1) Change nir_lower_source_to_mods so it does not try to rewrite alu > > operations where a source comes from a fmov with a negate, or at least > > if the instruction we are trying to rewrite is a vecN operation (or > > maybe allow this in scalar mode only?) > > > > 2) In nir_lower_vec_to_movs, if a source is negated, check its > > parent_instr and try to guess its type from that (in this example, we > > would see it came from fmov and we can say it is a float and emit fmov > > instead of imov). Not sure if this would work in all possible scenarios > > though. > > > > Opinions? > > > > Iago > > > > _______________________________________________ > > mesa-dev mailing list > > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> > > The only reason I changed vecN to produce floats is to avoid producing > 64-bit integer instructions, which at one point the constant folding > infrastructure couldn't support (but now it can), so you can just > revert the change. Ofc the i965 backend won't be able to express this > directly, but for now you can silently change 64-bit integers to > floats and assert that they only happen in things that copy data > around. I would tend to agree. We could also make it unsigned so no source modifiers ever make sense. Meh. If we did want to keep vecN float, the thing to do would be to make vec_to_move lower it to fmovs rather than imovs. But, like Connor said, just asserting no source modifiers for th 64-bit version in the backend is probably best. --Jason