Bug 109601

Created attachment 143352 [details]
RuneLite GPU rendering broken

Created attachment 143353 [details]
RuneLite GPU rendering working

I've also attached screenshots that demonstrate the issue.

Thanks a lot for the apitrace files.
Bisected to :

commit 4dd5263663585b55119e49c8dfade015c86aff1a (refs/bisect/bad)
Author: Jason Ekstrand <jason.ekstrand@intel.com>
Date:   Tue Aug 14 14:37:39 2018 -0500

    nir/algebraic: Add more extract_[iu](8|16) optimizations
    
    This adds the "(a << N) >> M" family of mask or sign-extensions.  Not a
    huge win right now but this pattern will soon be generated by NIR format
    lowering code.
    
    Shader-db results on Kaby Lake:
    
        total instructions in shared programs: 15166918 -> 15166916 (<.01%)
        instructions in affected programs: 36 -> 34 (-5.56%)
        helped: 2
        HURT: 0
    
    Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
    Reviewed-by: Kenneth Graunke <kenneth@whitecape.org>

The particular nir replacement that seems to cause issues is :

(('ishr', 'a@32', 16), ('extract_i16', a, 1), '!options->lower_extract_word')

I can't see anything wrong with it, so maybe there is an issue in our backend.

Jason: Any idea?

Some more digging shows that it seems to be a geometry shader has the issue.
Disabling the replacement only on that shader fixes the issue (this replacement is also used in a compute shader).
Here is the code of the geometry shader :

/*
 * Copyright (c) 2018, Adam <Adam@sigterm.info>
 * All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions are met:
 *
 * 1. Redistributions of source code must retain the above copyright notice, this
 *    list of conditions and the following disclaimer.
 * 2. Redistributions in binary form must reproduce the above copyright notice,
 *    this list of conditions and the following disclaimer in the documentation
 *    and/or other materials provided with the distribution.
 *
 * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
 * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
 * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
 * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
 * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
 * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
 * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
 * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
 * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */

#version 330

#define PI 3.1415926535897932384626433832795f
#define UNIT PI / 1024.0f

layout(triangles) in;
layout(triangle_strip, max_vertices = 3) out;

layout(std140) uniform uniforms {
  int cameraYaw;
  int cameraPitch;
  int centerX;
  int centerY;
  int zoom;
  int cameraX;
  int cameraY;
  int cameraZ;
  ivec2 sinCosTable[2048];
};

uniform mat4 projectionMatrix;

in ivec3 vPosition[];
in vec4 vColor[];
in float vHsl[];
in vec4 vUv[];
in float vFogAmount[];

out vec4 Color;
out float fHsl;
out vec4 fUv;
out float fogAmount;

/*
 * Copyright (c) 2018, Adam <Adam@sigterm.info>
 * All rights reserved.
 *
 * Redistribution and use in source and binary forms, with or without
 * modification, are permitted provided that the following conditions are met:
 *
 * 1. Redistributions of source code must retain the above copyright notice, this
 *    list of conditions and the following disclaimer.
 * 2. Redistributions in binary form must reproduce the above copyright notice,
 *    this list of conditions and the following disclaimer in the documentation
 *    and/or other materials provided with the distribution.
 *
 * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
 * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
 * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
 * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
 * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
 * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
 * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
 * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
 * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 */

/*
 * Convert a vertex to screen space
 */
ivec3 toScreen(ivec3 vertex, int cameraYaw, int cameraPitch, int centerX, int centerY, int zoom) {
  int yawSin = int(65536.0f * sin(cameraYaw * UNIT));
  int yawCos = int(65536.0f * cos(cameraYaw * UNIT));

  int pitchSin = int(65536.0f * sin(cameraPitch * UNIT));
  int pitchCos = int(65536.0f * cos(cameraPitch * UNIT));

  int rotatedX = ((vertex.z * yawSin) + (vertex.x * yawCos)) >> 16;
  int rotatedZ = ((vertex.z * yawCos) - (vertex.x * yawSin)) >> 16;

  int var13 = ((vertex.y * pitchCos) - (rotatedZ * pitchSin)) >> 16;
  int var12 = ((vertex.y * pitchSin) + (rotatedZ * pitchCos)) >> 16;

  int x = rotatedX * zoom / var12 + centerX;
  int y = var13 * zoom / var12 + centerY;
  int z = -var12; // in OpenGL depth is negative

  return ivec3(x, y, z);
}

void main() {
  ivec3 cameraPos = ivec3(cameraX, cameraY, cameraZ);
  ivec3 screenA = toScreen(vPosition[0] - cameraPos, cameraYaw, cameraPitch, centerX, centerY, zoom);
  ivec3 screenB = toScreen(vPosition[1] - cameraPos, cameraYaw, cameraPitch, centerX, centerY, zoom);
  ivec3 screenC = toScreen(vPosition[2] - cameraPos, cameraYaw, cameraPitch, centerX, centerY, zoom);

  if (-screenA.z < 50 || -screenB.z < 50 || -screenC.z < 50) {
    // the client does not draw a triangle if any vertex distance is <50
    return;
  }

  vec4 tmp = vec4(screenA.xyz, 1.0);
  Color = vColor[0];
  fHsl = vHsl[0];
  fUv = vUv[0];
  fogAmount = vFogAmount[0];
  gl_Position  = projectionMatrix * tmp;
  EmitVertex();

  tmp = vec4(screenB.xyz, 1.0);
  Color = vColor[1];
  fHsl = vHsl[1];
  fUv = vUv[1];
  fogAmount = vFogAmount[1];
  gl_Position  = projectionMatrix * tmp;
  EmitVertex();

  tmp = vec4(screenC.xyz, 1.0);
  Color = vColor[2];
  fHsl = vHsl[2];
  fUv = vUv[2];
  fogAmount = vFogAmount[2];
  gl_Position  = projectionMatrix * tmp;
  EmitVertex();

  EndPrimitive();
}

I've sent a patch to the ML which fixes this:

https://patchwork.freedesktop.org/patch/285517/

Unfortunately, it also seems to hang my GPU but that, I think, is unrelated.

should we have a piglit test for this case?

(In reply to Mark Janes from comment #8)
> should we have a piglit test for this case?

Yep!

I haven't had time to look at Jason's fix yet.
But I tried to build a piglit tests by dumping shifted values into an ssbo and the result made no sense at all.
Still need to debug.

The problem is if you have something like this:

> int y = something;
> float x = -(y >> 16);

Then the negation gets lost.  Likely, we'd have the same problem with

> int y = something;
> float x = abs(y >> 16);

or either of those with an unsigned integer.

(In reply to Jason Ekstrand from comment #10)
> The problem is if you have something like this:
> 
> > int y = something;
> > float x = -(y >> 16);
> 
> Then the negation gets lost.  Likely, we'd have the same problem with
> 
> > int y = something;
> > float x = abs(y >> 16);
> 
> or either of those with an unsigned integer.

Thanks a bunch : https://patchwork.freedesktop.org/patch/286177/

Fixed in master by the following commit:

commit 367b0ede4d9115aba772d6e46ec73642761f7ff6 (public/master)
Author: Jason Ekstrand <jason.ekstrand@intel.com>
Date:   Mon Feb 11 22:39:45 2019 -0600

    intel/fs: Bail in optimize_extract_to_float if we have modifiers
    
    This fixes a bug in runscape where we were optimizing x >> 16 to an
    extract and then negating and converting to float.  The NIR to fs pass
    was dropping the negate on the floor breaking a geometry shader and
    causing it to render nothing.
    
    Fixes: 1f862e923cb "i965/fs: Optimize float conversions of byte/word..."
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109601
    Tested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
    Reviewed-by: Matt Turner <mattst88@gmail.com>