On Mon, Nov 9, 2015 at 10:41 AM, Jason Ekstrand <ja...@jlekstrand.net> wrote: > > On Nov 9, 2015 7:24 AM, "Connor Abbott" <cwabbo...@gmail.com> wrote: >> >> On Mon, Nov 9, 2015 at 6:55 AM, Iago Toral <ito...@igalia.com> 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 >> > mesa-dev@lists.freedesktop.org >> > http://lists.freedesktop.org/mailman/listinfo/mesa-dev >> >> 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.
Oh yeah, I meant assert that we don't get e.g. a 64-bit iadd, so we remember to fix that later. When we get support for real 64-bit integers, we'll have to only map nir_type_int64/uint64 to DF on gen7. > > 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 _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
