[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 #8 from Connor Abbott <cwabbott0 at gmail.com> ---
(In reply to Iago Toral from comment #6)
> While having a look at the patches I noticed a couple of obvious bugs that I
> thought would break things outside fp64, so I ran piglit and found that this
> in fact breaks a ton of things on my IVB laptop. A run of shader.py (without
> fp64 tests) gives:
> 
> [14079/14079] skip: 7034, pass: 5104, fail: 1046, crash: 895 \       
> 
> including a handful GPU hangs :(
> 
> I have been fixing a lot of these today, mostly small things here and there
> and now we are at:
> 
> [14079/14079] skip: 6973, pass: 7043, fail: 46, crash: 3
> 
> This means a total of 48 regressions compared to master excluding fp64
> functionality (and only for shader.py). These regressions include 9 GPU
> hangs.
> 
> In case you want to have a look, I have uploaded my fixes to this branch
> (rebased against current master too):
> https://github.com/Igalia/mesa/commits/connor-i965-fp64-v3-rebased
> 
> I think we should focus on tracking and fixing the remaining regressions
> before moving on to doing actual fp64 stuff.
> 
> BTW, right now we have things like nir_type_float and nir_type_float32, or
> nit_type_int and nir_type_int32...there is a number of patches in your
> branch that changes some uses of basic types by the bit-sized versions, and
> I had to add a few more in my branch to fix a good number of crashes and
> problems since the driver has some code paths that expect a bit-sized type
> in any case (in fact, they have assertions for this). Why don't we just drop
> the types that don't have the bit-size information completely? As it is now
> I think they only cause confusion and bugs.

No... in fact, the unsized types were kind of the entire point of Jason's
original proposal that I took as the basis of my implementation. A type of
nir_type_float means "this instruction can take any bit-width, but it has to
match with any other source/destination of this instruction that is
nir_type_float as well," sort of like how an input_size/output_size of 0 allows
to parameterize on vector width. For example, the new definition of fadd is:

binop("fadd", tfloat, commutative + associative, "src0 + src1")

which mean that output_type, input_type[0], and input_type[1] are all
nir_type_float. The same opcode is used for f32 and f64, and when we add an f16
type and a mediump/relaxed-precision "f24" type, not much will have to change.

Now, if there are asserts we're missing to catch these sorts of things, then by
all means go and add them. But having a nir_type_float around to mean "any
width float will do" is one of the central points of the whole idea. Maybe we
could rename it to "nir_type_float_all" or something?

BTW, for the GPU hangs, some of them might be caused by this commit:

http://cgit.freedesktop.org/~cwabbott0/mesa/commit/?h=i965-fp64-v3&id=1b38ed4e53da1a6a3e4ca1d659ce52f159e16037

it removes an old hack from back when we used to guess the exec_size of an
instruction which breaks fp64, but I wouldn't be surprised if removing it
uncovers other hidden bugs. I didn't manage to test it (I think Jenkins was
down when I tried last summer), but it would be nice to fix any regressions and
get that merged ahead of the rest of the series.

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