Bug 109353

Summary:	[regression][bisected] "nir: Switch to using 1-bit Booleans for almost everything" regression with shared bools
Product:	Mesa	Reporter:	Alejandro Piñeiro (freenode IRC: apinheiro) <apinheiro>
Component:	Drivers/DRI/i965	Assignee:	Intel 3D Bugs Mailing List <intel-3d-bugs>
Status:	RESOLVED FIXED	QA Contact:	Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity:	normal
Priority:	medium	CC:	apinheiro, jason, sergii.romantsov
Version:	unspecified
Hardware:	Other
OS:	All
Whiteboard:
i915 platform:		i915 features:
Attachments:	vkrunner test that reproduces the crash 109353_ext

Description Alejandro Piñeiro (freenode IRC: apinheiro) 2019-01-14 14:28:58 UTC

Since the following commit:

"commit 44227453ec03f5462f1cff5760909a9dba95c61a
Author: Jason Ekstrand <jason.ekstrand@intel.com>
Date:   Fri Oct 19 11:14:47 2018 -0500

    nir: Switch to using 1-bit Booleans for almost everything
    
    This is a squash of a few distinct changes:
    
        glsl,spirv: Generate 1-bit Booleans
    
        Revert "Use 32-bit opcodes in the NIR producers and optimizations"
    
        Revert "nir/builder: Generate 32-bit bool opcodes transparently"
    
        nir/builder: Generate 1-bit Booleans in nir_build_imm_bool
    
    Reviewed-by: Eric Anholt <eric@anholt.net>
    Reviewed-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>
    Tested-by: Bas Nieuwenhuizen <bas@basnieuwenhuizen.nl>"

Using a shared bool on a compute shader raises the following assert:
  SPIR-V parsing FAILED:
    In file ../src/compiler/spirv/vtn_variables.c:406
    base->ptr_type->length > 0 && base->ptr_type->align > 0
    1028 bytes into the SPIR-V binary
shader_runner: ../src/mesa/main/glspirv.c:268: _mesa_spirv_to_nir: Assertion `entry_point' failed.
Aborted (core dumped)

Although I initially found this while testing ARB_gl_spirv, I confirmed that it also happens on anv Vulkan. I will submit a vkrunner test soon.

Comment 1 Alejandro Piñeiro (freenode IRC: apinheiro) 2019-01-14 14:33:17 UTC

Created attachment 143106 [details]
vkrunner test that reproduces the crash

This is just the compute shader example at vkrunner, but adding a shared bool.

Not sure if it makes sense to add it to piglit. I could do it if anyone thinks that it makes sense.

Comment 2 Sergii Romantsov 2019-01-15 11:09:50 UTC

Patch: https://patchwork.freedesktop.org/patch/277460/

Comment 3 Sergii Romantsov 2019-01-15 11:12:54 UTC

Created attachment 143123 [details]
109353_ext

Hello, Alejandro
Seems in case of piglit-test, it seems better to extend it as 109353_ext - it handles additional sub-case.

Comment 4 Alejandro Piñeiro (freenode IRC: apinheiro) 2019-01-15 11:35:22 UTC

(In reply to Sergii Romantsov from comment #3)
> Created attachment 143123 [details]
> 109353_ext
> 
> Hello, Alejandro
> Seems in case of piglit-test, it seems better to extend it as 109353_ext -
> it handles additional sub-case.

Looks good to me. Although again, not sure if it makes sense to add just one regression test to piglit.

Comment 5 Jason Ekstrand 2019-01-23 21:44:08 UTC

This should be fixed by the following commit in master:

commit cfca5cd9588fb213e6889c85137f3e2fec8c7757 (public/master)
Author: Sergii Romantsov <sergii.romantsov@gmail.com>
Date:   Tue Jan 15 13:08:32 2019 +0200

    nir: Length of boolean vtn_value now is 1
    
    During conversion type-length was lost due to math.
    
    v2 (Jason Ekstrand):
     - Use a size/offset of 4 bytes
    
    Fixes: 44227453ec03 (nir: Switch to using 1-bit Booleans for almost everything)
    Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109353
    Signed-off-by: Sergii Romantsov <sergii.romantsov@globallogic.com>
    Tested-by: Alejandro Piñeiro <apinheiro@igalia.com>
    Reviewed-by: Alejandro Piñeiro <apinheiro@igalia.com>
    Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>

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.