Bug 107524 - Broken packDouble2x32 at llvmpipe
Summary: Broken packDouble2x32 at llvmpipe
Status: RESOLVED FIXED
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/Gallium/llvmpipe (show other bugs)
Version: git
Hardware: All Linux (All)
: medium normal
Assignee: mesa-dev
QA Contact: mesa-dev
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-08-08 16:14 UTC by Matwey V. Kornilov
Modified: 2018-12-07 13:54 UTC (History)
3 users (show)

See Also:
i915 platform:
i915 features:


Attachments
shader_run test (983 bytes, text/plain)
2018-08-10 08:08 UTC, Matwey V. Kornilov
Details
pass two swizzles into fetches. (17.52 KB, patch)
2018-08-27 01:51 UTC, Dave Airlie
Details | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Matwey V. Kornilov 2018-08-08 16:14:13 UTC
I've tested Mesa llvmpipe from recent master (e02f061b690d) and found the following behavior.

The following fragment shader works as expected on Intel GPU (Intel HD Graphics 620 (Kaby Lake GT2)) and shows black screen at llvmpipe.

        #version 330
        #extension GL_ARB_gpu_shader_fp64 : require
        in vec2 UV;
        out vec4 color;
        uniform usampler2D image_texture;
        uniform sampler1D colormap;
        uniform float c;
        uniform float z;

        void main() {
                double raw_value = packDouble2x32(texture(image_texture, UV).gr);
                double value = c * (raw_value - z);
                color = texture(colormap, float(clamp(value, 0.0, 1.0)));
        }

At the same time, the following tiny modification fixes the shader at llvmpipe:

        #version 330
        #extension GL_ARB_gpu_shader_fp64 : require
        in vec2 UV;
        out vec4 color;
        uniform usampler2D image_texture;
        uniform sampler1D colormap;
        uniform float c;
        uniform float z;

        void main() {
                double raw_value = packDouble2x32(uvec2(texture(image_texture, UV).g, texture(image_texture, UV).r));
                double value = c * (raw_value - z);
                color = texture(colormap, float(clamp(value, 0.0, 1.0)));
        }

I've compared TGSI output from both of them and found the following difference:
In broken case DADD tries to operate on .yxyx

  5: DADD TEMP[1].xy, TEMP[1].yxyx, TEMP[2].xyxy

I am not sure that DADD knows what to do with swapped yx.
Comment 1 Roland Scheidegger 2018-08-08 17:07:59 UTC
I think your analysis is correct, gallivm code cannot handle this (I believe it should work with softpipe?).
I'm not too familiar with the double support, in particular I'm not entirely sure if such swizzles are even supposed to be supported for double opcodes, though I don't really see a compelling reason why not (otherwise could of course always get rid of them with a mov).
But at a quick glance, lp_build_tgsi_inst_llvm() will ignore the second and fourth channel when fetching the arguments, emit_fetch_temporary() and others will instead for 64bit src do a second fetch themselves, however they will always use swizzle + 1 for it. This probably means that when the swizzle is .yx it will instead actually fetch .yz for the double src (if you'd try .wz I think you'd get an assertion failure).
I don't quite know how to fix this in some elegant way (as those fetch args are propagated through lots of function, so those emit_fetch_temporary() and friends functions simply have no knowledge what the second channel actually should be for 64bit opcodes). Maybe Dave has some idea? I think one way or another (at least if such swizzles are considered valid), the swizzle for the second source has to be propagated through explicitly.
Comment 2 Dave Airlie 2018-08-08 22:50:33 UTC
Having a piglit shader_runner example test for this would be nice, if you had some time to make one.
Comment 3 Matwey V. Kornilov 2018-08-10 08:08:37 UTC
Created attachment 141033 [details]
shader_run test

Attached is what are you looking for.

It passes at Intel and fails at llvmpipe.
Comment 4 Matwey V. Kornilov 2018-08-24 12:18:52 UTC
Any news?
Comment 5 Dave Airlie 2018-08-26 23:38:55 UTC
Looked at it a bit last week, but it's messier than I thought, need to dig a bit more to find a proper answer.
Comment 6 Dave Airlie 2018-08-27 01:51:45 UTC
Created attachment 141293 [details] [review]
pass two swizzles into fetches.
Comment 7 Dave Airlie 2018-08-27 01:52:00 UTC
The attached patch shoulw in theory fix it.
Comment 8 Matwey V. Kornilov 2018-08-27 08:38:50 UTC
(In reply to Dave Airlie from comment #6)
> Created attachment 141293 [details] [review] [review]
> pass two swizzles into fetches.

I've checked that the patch fixes the issue for llvm at my side.
Comment 9 Emil Velikov 2018-12-07 13:54:59 UTC
Should be fixed with the following commit. Feel free to reopen otherwise.

commit bb17ae49ee2591a4a35479ed6e48cb3c18422e2a
Author: Dave Airlie <airlied@redhat.com>
Date:   Mon Aug 27 02:03:41 2018 +0100

    gallivm: allow to pass two swizzles into fetches.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.