[Bug 92760] Add FP64 support to the i965 shader backends

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=92760

--- Comment #92 from Francisco Jerez <currojerez at riseup.net> ---
(In reply to Juan A. Suarez from comment #91)
> (In reply to Francisco Jerez from comment #89)
> > (In reply to Juan A. Suarez from comment #88)
> > >[..]
> > > Related with this topic, current liveness analysis code (and also DCE
> > > optimization) just ignore the types when dealing with the swizzles. This
> > > works fine, except when reading/writing a channel from DF type (64-bit
> > > channel), and write/read it later as F (32-bit channel).
> > > 
> > > For instance, for this piece is code:
> > > 
> > >     mov(8) vgrf3.0.x:UD[1], 2576980378U[1] NoMask
> > >     mov(8) vgrf3.0.y:UD[1], 1071225241U[1] NoMask
> > >     mov(8) vgrf3.1.x:UD[1], 2576980378U[1] NoMask
> > >     mov(8) vgrf3.1.y:UD[1], 1071225241U[1] NoMask
> > >     mov(8) vgrf2.0.xy:DF[2], vgrf3.0.xxxx:DF[2]
> > > 
> > > Our liveness analysis just decide vgrf3.0.y channel is not live anymore and
> > > thus DCE removes it. That is because in latest instruction it doesn't
> > > realize vgrf3.0.x:DF is reading both vgrf3.0.x:F and vgrf3.0.y:F.
> > > 
> > > In order to avoid introducing too many changes in our current code, I've
> > > added a pair of commits[1][2] that basically try to check if a 32-bit
> > > channels was previously read as 64-bit (and the other way around). This
> > > seems to fix above problem.
> > 
> > Seems very sketchy.  You need to fix the current dataflow analysis logic to
> > flip/check as many bits per channel from the live, use or def bitsets as the
> > corresponding source or destination datatype takes, just like the FS back-end
> > handles multi-GRF defs and uses.
> > 
> 
> Thanks for the feedback!
> 
> I threw away the changes done, and re-did it following your suggestion. Now,
> it flips/checks as many bits as the datatypes require.
> 
> I find this implementation more easy to understand.
> 
> Commit is in
> https://github.com/Igalia/mesa/commit/
> 59fd1cc6b99dc993e0da772674a57390a54955f3

Looks somewhat better, but we need to settle on some well-defined meaning for
the bits of the live/defs/uses sets and stick to it consistently, you cannot
interpret the live bitsets as either eight or four bits per GRF depending on
the instruction and expect things to work smoothly.  The meaning of the live
bitsets should be independent from the instructions writing or reading from
them -- The dataflow tracking logic is already complicated enough to have
different sections of the same live bitset representing variable liveness
differently.

The previous implementation used to represent live sets in terms of logical
32-bit components, which breaks down when you use non-32-bit types in the
program, or when you do any kind of SIMD splitting.  It would be possible to
support DF code with only four bits per GRF by redefining the current
representation slightly, so bit '4*j + i' of the live set would refer to the
logical OR of the liveness of the i-th and i+4-th dwords of the same GRF, but
that would cause the live intervals calculated for DF instructions to be less
accurate, and it would be rather awkward to implement -- What you've done
instead allocating eight bits per GRF looks fine (you'd have each bit represent
one physical dword of the GRF), but you need to initialize (or check) the whole
eight bits of each GRF written (or read) by the instruction regardless of
whether the instruction is double- or single-precision.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/intel-3d-bugs/attachments/20160617/d68a53f1/attachment.html>