ValveSoftware / csgo-osx-linux

Counter-Strike: Global Offensive
http://counter-strike.net
785 stars 69 forks source link

Artifacts on de_cache when shader level is "high" or "very high (default)" #1567

Closed Heis closed 6 years ago

Heis commented 6 years ago

Your system information

Computer Information: Manufacturer: Unknown Model: Unknown Form Factor: Desktop No Touch Input Detected

Processor Information: CPU Vendor: AuthenticAMD CPU Brand: AMD Ryzen 5 1600X Six-Core Processor
CPU Family: 0x17 CPU Model: 0x1 CPU Stepping: 0x1 CPU Type: 0x0 Speed: 3600 Mhz 12 logical processors 6 physical processors HyperThreading: Supported FCMOV: Supported SSE2: Supported SSE3: Supported SSSE3: Supported SSE4a: Supported SSE41: Supported SSE42: Supported AES: Supported AVX: Supported CMPXCHG16B: Supported LAHF/SAHF: Supported PrefetchW: Unsupported

Operating System Version: Linux 4.x (64 bit) Kernel Name: Linux Kernel Version: 4.14.4-1-ARCH X Server Vendor: The X.Org Foundation X Server Release: 11905000 X Window Manager: KWin Steam Runtime Version: steam-runtime-beta-release_2017-07-24

Video Card: Driver: X.Org Radeon RX 580 Series (AMD POLARIS10 / DRM 3.19.0 / 4.14.4-1-ARCH, LLVM 5.0.0) Driver Version: 3.0 Mesa 17.2.5 OpenGL Version: 3.0 Desktop Color Depth: 24 bits per pixel Monitor Refresh Rate: 144 Hz VendorID: 0x1002 DeviceID: 0x67df Revision Not Detected Number of Monitors: 1 Number of Logical Video Cards: 1 Primary Display Resolution: 1920 x 1080 Desktop Resolution: 1920 x 1080 Primary Display Size: 20.91" x 11.77" (23.98" diag) 53.1cm x 29.9cm (60.9cm diag) Primary VRAM: 8171 MB

Sound card: Audio device: ATI R6xx HDMI

Memory: RAM: 16045 Mb

Miscellaneous: UI Language: English LANG: en_US.UTF-8 Total Hard Disk Space Available: 468427 Mb Largest Free Hard Disk Block: 413929 Mb VR Headset: None detected

Recent Failure Reports:

Please describe your issue in as much detail as possible:

There are artifacts in the rendered ground (so far I have only seen it on de_cache). Two offset black squares with glitches appear around the player especially in t-spawn and on A site.

de_cache0005 de_cache0006 de_cache0007 de_cache0008 de_cache0009

Steps for reproducing this issue:

  1. Join any game on de_cache
  2. Set shader level to "high" or "very high (default)"
  3. Look at artifacts in rendering the ground especially in t-spawn and A site.
kisak-valve commented 6 years ago

Hello @Heis, on my AMD test box, mesa 17.2.6 built against llvm 5.0.0 is affected, while llvm 4.0.1 is not affected.

This should also be brought to the attention of your driver vendor if it has not been already.

Heis commented 6 years ago

Ok! so i do not undersrtand the mechanisms behind this issue. I would like to report this upstream. Where to report it, though?

kisak-valve commented 6 years ago

For mesa, there's some info at https://www.mesa3d.org/bugs.html. Mesa/radeonsi uses llvm to compile shaders for the gpu.

Heis commented 6 years ago

Mesa bug report can be found here: https://bugs.freedesktop.org/show_bug.cgi?id=104190

Thanks for the help!

kisak-valve commented 6 years ago

It's worth noting mesa-git + llvm-git from today is also not affected.

kisak-valve commented 6 years ago

Hello @kvbev, I threw Mesa 17.3.1 built against LLVM 5.0.1 on my testbox and the issue is present.

Heis commented 6 years ago

Hi! I tested yesterday, and can confirm the problem is still there.

jpotier commented 6 years ago

Same problem, still current.

https://framapic.org/kzu6d6vYAAOo/g4vRdfKc9hEo.png

kisak-valve commented 6 years ago

This issue bisects to

commit 5d860d251edd2336d8542c7e60b062186dff1258
Author: Keno Fischer <keno@alumni.harvard.edu>
Date:   Fri Jun 9 19:31:10 2017 +0000

    [Sink] Fix predicate in legality check

    Summary:
    isSafeToSpeculativelyExecute is the wrong predicate to use here.
    All that checks for is whether it is safe to hoist a value due to
    unaligned/un-dereferencable accesses. However, not only are we doing
    sinking rather than hoisting, our concern is that the location
    we're loading from may have been modified. Instead forbid sinking
    any load across a critical edge.

    Reviewers: majnemer

    Subscribers: davide, llvm-commits

    Differential Revision: https://reviews.llvm.org/D33179

    git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@305102 91177308-0d34-0410-b5e6-96231b3b80d8

:040000 040000 78f0d91acd5074e5013b6704cd9878860d7ca61b 9b64356ee6435aac2dba7d5d584a38905a3af522 M  lib
:040000 040000 5668c19e9514128afc9a8bd7b1c8e5d1828851bf 58651ffd91198cba48083ae82aa7bfa250187b3e M  test

in llvm.

kisak-valve commented 6 years ago

The fix for this issue bisects to llvm commit:

commit ddb10d2e51d6a55b9cff98797d275163ed14a5c3
Author: Stanislav Mekhanoshin <Stanislav.Mekhanoshin@amd.com>
Date:   Wed Jul 26 21:29:15 2017 +0000

    [AMDGPU] Optimize SI_IF lowering for simple if regions

    Currently SI_IF results in a s_and_saveexec_b64 followed by s_xor_b64.
    The xor is used to extract only the changed bits. In case of a simple
    if region where the only use of that value is in the SI_END_CF to
    restore the old exec mask, we can omit the xor and perform an or of
    the exec mask with the original exec value saved by the
    s_and_saveexec_b64.

    Differential Revision: https://reviews.llvm.org/D35861

    git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@309185 91177308-0d34-0410-b5e6-96231b3b80d8

:040000 040000 b5f8f665583c3dc514e0f7d436a03d62bedb07ca cf1b204080c77f81cdcc6f64200d8eac1bcadb97 M  lib
:040000 040000 76c1919aaf28180e1db4f8f29dde07855aa9a617 189f7f95523f6c948e46fb599f721f4f4c889b22 M  test

and mesa commit:

commit 2f4705afde707e8eb41b9414c25df91aa1ea2fb3
Author: Marek Olšák <marek.olsak@amd.com>
Date:   Sun Oct 8 03:44:07 2017 +0200

    radeonsi: if there's just const buffer 0, set it in place of CONST/SSBO pointer

    SI_SGPR_CONST_AND_SHADER_BUFFERS now contains the pointer to const buffer 0
    if there is no other buffer there.

    Benefits:
    - there is no constbuf descriptor upload and shader load

    It's assumed that all constant addresses are within bounds. Non-constant
    addresses are clamped against the last declared CONST variable.
    This only works if the state tracker ensures the bound constant buffer
    matches what the shader needs.

    Once we get 32-bit pointers, we can only do this for user constant buffers
    where the driver is in charge of the upload so that it can guarantee a 32-bit
    address.

    The real performance benefit might not be measurable.

    These apps get 100% theoretical benefit in all shaders (except where noted):
    - antichamber
    - barman arkham origins
    - borderlands 2
    - borderlands pre-sequel
    - brutal legend
    - civilization BE
    - CS:GO
    - deadcore
    - dota 2 -- most shaders
    - europa universalis
    - grid autosport -- most shaders
    - left 4 dead 2
    - legend of grimrock
    - life is strange
    - payday 2
    - portal
    - rocket league
    - serious sam 3 bfe
    - talos principle
    - team fortress 2
    - thea
    - unigine heaven
    - unigine valley -- also sanctuary and tropics
    - wasteland 2
    - xcom: enemy unknown & enemy within
    - tesseract
    - unity (engine)

    Changed stats only:
        SGPRS: 2059998 -> 2086238 (1.27 %)
        VGPRS: 1626888 -> 1626904 (0.00 %)
        Spilled SGPRs: 7902 -> 7865 (-0.47 %)
        Code Size: 60924520 -> 60982660 (0.10 %) bytes
        Max Waves: 374539 -> 374526 (-0.00 %)

    Reviewed-by: Nicolai Hähnle <nicolai.haehnle@amd.com>

:040000 040000 7b660ccfb110c80ecd9b3d2f7f06ace5380cb76f d8ddc8515150a84131e566c4ddafb05d3d7be0a7 M  src

Building llvm 5.0.1 with D35861 fixes this issue. Tested with Mesa 17.3.3.

kisak-valve commented 6 years ago

Closing as another LLVM 5.0 release with this fixed is unlikely. The mesa side was also closed with this evaluation.