Bug 109075

Polaris 30 (RX 590, 4.20 and latest firmware), DXVK 0.94
Latest mesa-git built against llvm 7.0.1

This line also seems to break Overwatch, resulting in a black login screen.

Commenting out the line Philip mentioned fixed it.

This is really weird because, of all the optimizations in that commit, that is the one that's NOT new.  It wasn't in that exact form but we definitely had that optimization before my 1-bit booleans series.  Maybe some other change made it suddenly trigger in a new place and cascade into additional optimization?

Maybe getting stuck in a infinite loop?

There is a loop exit condition in the witcher where the optimise the following: 

		/* preds: block_3 */
		vec2 32 ssa_114 = vec2 ssa_105, ssa_4
		vec4 32 ssa_115 = txf ssa_95 (texture_deref), ssa_114 (coord), ssa_4 (lod), 0 (sampler), 
		vec1 1 ssa_120 = flt ssa_115.x, ssa_93
		vec1 1 ssa_121 = flt ssa_115.y, ssa_94
		vec1 32 ssa_122 = b2i32 ssa_120
		vec1 32 ssa_123 = ineg ssa_122
		vec1 32 ssa_125 = b2i32 ssa_121
		vec1 32 ssa_126 = ineg ssa_125
		vec1 1 ssa_128 = flt ssa_93, ssa_115.z
		vec1 1 ssa_129 = flt ssa_94, ssa_115.w
		vec1 32 ssa_130 = b2i32 ssa_128
		vec1 32 ssa_131 = ineg ssa_130
		vec1 32 ssa_133 = b2i32 ssa_129
		vec1 32 ssa_134 = ineg ssa_133
		vec1 32 ssa_136 = iand ssa_123, ssa_131
		vec1 32 ssa_137 = iand ssa_126, ssa_136
		vec1 32 ssa_138 = iand ssa_134, ssa_137
		vec1 1 ssa_139 = ine ssa_138, ssa_4

to:

                vec2 32 ssa_87 = vec2 ssa_84, ssa_4
                vec4 32 ssa_88 = txf ssa_82 (texture_deref), ssa_87 (coord), ssa_4 (lod), 0 (sampler),
                vec1 1 ssa_89 = flt ssa_88.x, ssa_80
                vec1 1 ssa_90 = flt ssa_88.y, ssa_81
                vec1 32 ssa_91 = b2i32 ssa_89
                vec1 32 ssa_92 = ineg ssa_91
                vec1 32 ssa_93 = b2i32 ssa_90
                vec1 32 ssa_94 = ineg ssa_93
                vec1 1 ssa_95 = flt ssa_80, ssa_88.z
                vec1 1 ssa_96 = flt ssa_81, ssa_88.w
                vec1 32 ssa_97 = b2i32 ssa_95
                vec1 32 ssa_98 = ineg ssa_97
                vec1 32 ssa_99 = b2i32 ssa_96
                vec1 32 ssa_100 = ineg ssa_99
                vec1 1 ssa_101 = iand ssa_89, ssa_95
                vec1 32 ssa_102 = b2i32 ssa_101
                vec1 32 ssa_103 = ineg ssa_102
                vec1 32 ssa_104 = imov ssa_103
                vec1 32 ssa_105 = b2f32 ssa_90
                vec1 32 ssa_106 = imov ssa_105
                vec1 32 ssa_107 = b2f32 ssa_96
                vec1 32 ssa_108 = imov ssa_107
                vec1 1 ssa_109 = ine ssa_108, ssa_4

Which ends up as:

		vec2 32 ssa_71 = vec2 ssa_69, ssa_4
		vec4 32 ssa_72 = txf ssa_67 (texture_deref), ssa_71 (coord), ssa_4 (lod), 0 (sampler), 
		vec1 32 ssa_73 = flt32 ssa_66, ssa_72.w
		vec1 32 ssa_74 = b2f32 ssa_73
		vec1 32 ssa_75 = ine32 ssa_74, ssa_4

With the offending opt commented out we end up with:

		vec2 32 ssa_76 = vec2 ssa_74, ssa_4
		vec4 32 ssa_77 = txf ssa_72 (texture_deref), ssa_76 (coord), ssa_4 (lod), 0 (sampler), 
		vec1 32 ssa_78 = flt32 ssa_77.x, ssa_70
		vec1 32 ssa_79 = flt32 ssa_77.y, ssa_71
		vec1 32 ssa_80 = flt32 ssa_70, ssa_77.z
		vec1 32 ssa_81 = flt32 ssa_71, ssa_77.w
		vec1 32 ssa_82 = iand ssa_78, ssa_80
		vec1 32 ssa_83 = iand ssa_79, ssa_82
		vec1 32 ssa_84 = iand ssa_81, ssa_83

It looks like the '1.0@32' part of the search is failing. It also seems this is the only opt that looks for a constant in the whole file. Everything else looks for variables or ops e.g. 'a@32', 'bcsel@32'

The problem is the quotation marks around 1.0 cause it to be treated as a string instead of a floating point value.  The generator then treats it as an arbitrary variable replacement, so any iand involving a ('ineg', ('b2i', a)) matches.

I have a fix, and I'll submit an MR in a couple minutes.

Can you try https://gitlab.freedesktop.org/mesa/mesa/merge_requests/30 to see if that fixes the problem?

Test case sent to the piglit list: https://patchwork.freedesktop.org/patch/268702/

(In reply to Ian Romanick from comment #5)
> The problem is the quotation marks around 1.0 cause it to be treated as a
> string instead of a floating point value.  The generator then treats it as
> an arbitrary variable replacement, so any iand involving a ('ineg', ('b2i',
> a)) matches.
> 

Yes but as per my comment in the merge request that is meant to be supported.

The patch that Ian posted fixes both the Witcher 3 hang and the Overwatch issue.

Fixed by:

commit 96c4b135e34d0804e41bfbc28fc1b5050c49d71e (HEAD -> bug-109075, gitlab/bug-109075)
Author: Ian Romanick <ian.d.romanick@intel.com>
Date:   Tue Dec 18 13:28:22 2018 -0800

    nir/algebraic: Don't put quotes around floating point literals
    
    The quotation marks around 1.0 cause it to be treated as a string
    instead of a floating point value.  The generator then treats it as an
    arbitrary variable replacement, so any iand involving a ('ineg', ('b2i',
    a)) matches.
    
    v2: Remove misleading comment about sized literals (suggested by
    Timothy).  Add assertion that the name of a varible is entierly
    alphabetic (suggested by Jason).
    
    Signed-off-by: Ian Romanick <ian.d.romanick@intel.com>
    Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
    Tested-by: Timothy Arceri <tarceri@itsqueeze.com> [v1]
    Reviewed-by: Timothy Arceri <tarceri@itsqueeze.com> [v1]
    Fixes: 6bcd2af0863 ("nir/algebraic: Add some optimizations for D3D-style Booleans")
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109075