Matrix multiplication problems

[ssvb]

Division by zero trying to compile the following simple vertex shader:

    uniform mat4 modelviewprojectionMatrix;

    attribute vec4 in_position;  
    attribute vec2 in_coord;     

    varying vec2 coord;          

    void main()                  
    {                            
        gl_Position = modelviewprojectionMatrix * in_position;
        coord = in_coord;        
    }

==27873== Process terminating with default action of signal 8 (SIGFPE)
==27873== Integer divide by zero at address 0x4049E9641
==27873== at 0x40346B: get_hash (ptrset.c:41)
==27873== by 0x40376D: ptrset_add (ptrset.c:114)
==27873== by 0x412DDF: lima_gp_ir_node_link (node.c:674)
==27873== by 0x454432: (anonymous namespace)::gp_ir_visitor::emit_uniform_load(ir_dereference_, bool) (from_glsl.cpp:1222)
==27873== by 0x453CAC: (anonymous namespace)::gp_ir_visitor::handle_deref(ir_dereference_) (from_glsl.cpp:1001)
==27873== by 0x453AC8: (anonymous namespace)::gp_ir_visitor::visit_enter(ir_dereference_array_) (from_glsl.cpp:950)
==27873== by 0x485436: ir_dereference_array::accept(ir_hierarchical_visitor_) (ir_hv_accept.cpp:272)
==27873== by 0x45368C: (anonymous namespace)::gp_ir_visitor::emit_expression(lima_gp_ir_op_e, ir_rvalue**, unsigned int) (from_glsl.cpp:855)
==27873== by 0x452D42: (anonymous namespace)::gp_ir_visitor::visit_enter(ir_expression_) (from_glsl.cpp:616)
==27873== by 0x485249: ir_expression::accept(ir_hierarchical_visitor_) (ir_hv_accept.cpp:150)
==27873== by 0x45368C: (anonymous namespace)::gp_ir_visitor::emit_expression(lima_gp_ir_op_e, ir_rvalue**, unsigned int) (from_glsl.cpp:855)
==27873== by 0x452D1F: (anonymous namespace)::gp_ir_visitor::visit_enter(ir_expression*) (from_glsl.cpp:612)

[cwabbott0]

I'm not sure why you're getting a divide-by-zero error (on my machine, I get a segfault) but it seems that the underlying issue is that GLSL IR is lowering matrix operations like

modelviewprojectionMatrix * in_position

to

modelviewprojectionMatrix[0] * in_position.x + modelviewprojectionMatrix[1] * in_position.y + ...

which is good, except it seems to represent modelviewprojectionMatrix[0] as an ir_dereference_array, even though the thing being dereferenced isn't an array, which confuses us when we lower dereferences to offsets since we don't handle that case yet.

[ssvb]

Can this shader be modified into something that is currently supported?

[cwabbott0]

Well, if you handcoded those operations instead of using matrices it should work... I'll try and fix the bug tonight though, so hopefully you won't have to. It's nice having people test this, even if the result is "oh whoops, I forgot to handle matrices!"

[cwabbott0]

Since 6a24095, we now generate correct GP IR for this shader, and valgrind gives me no errors, so this particular error shouldn't happen for you. However, for some reason, the scheduler keeps restarting without making any progress until it runs out of registers to be allocated the fast way, at which point it hits an assert since we haven't implemented the slow path (which involves generating driver-internal uniforms to hold constants, something we couldn't do until pretty recently). The IR before this seems sane, again valgrind shows no errors, and the scheduler should have no problem scheduling this, so there's one of 2 things happening:

1. There's some subtle bug in the scheduler or earlier code that valgrind isn't catching.
2. Our scheduling heuristics are somehow off and need some tweaking.

Either way, it's going to be a lot harder to debug... sorry.

[ssvb]

After trying to play around with the code and change mat4/vec4 to mat3/vec3, it segfaults again:

uniform mat3 modelviewprojectionMatrix;

attribute vec3 in_position;
attribute vec2 in_coord;

varying vec2 coord;

void main()
{
    vec3 tmp = modelviewprojectionMatrix * in_position;
    gl_Position = vec4(tmp.x, tmp.y, tmp.z, 1.0);
    coord = in_coord;
}

[ssvb]

Well, if you handcoded those operations instead of using matrices it should work... I'll try and fix > > the bug tonight though, so hopefully you won't have to. It's nice having people test this, even if the > result is "oh whoops, I forgot to handle matrices!"

FWIW, handcoding the matrix operation indeed works now at least in 51f162b:

uniform mat4 modelviewprojectionMatrix;

attribute vec4 in_position;
attribute vec2 in_coord;

varying vec2 coord;

void main()
{
    gl_Position = vec4(modelviewprojectionMatrix[0][0] * in_position.x +
                       modelviewprojectionMatrix[1][0] * in_position.y +
                       modelviewprojectionMatrix[2][0] * in_position.z +
                       modelviewprojectionMatrix[3][0] * in_position.w,

                       modelviewprojectionMatrix[0][1] * in_position.x +
                       modelviewprojectionMatrix[1][1] * in_position.y +
                       modelviewprojectionMatrix[2][1] * in_position.z +
                       modelviewprojectionMatrix[3][1] * in_position.w,

                       modelviewprojectionMatrix[0][2] * in_position.x +
                       modelviewprojectionMatrix[1][2] * in_position.y +
                       modelviewprojectionMatrix[2][2] * in_position.z +
                       modelviewprojectionMatrix[3][2] * in_position.w,

                       modelviewprojectionMatrix[0][3] * in_position.x +
                       modelviewprojectionMatrix[1][3] * in_position.y +
                       modelviewprojectionMatrix[2][3] * in_position.z +
                       modelviewprojectionMatrix[3][3] * in_position.w);
    coord = in_coord;
}

[cwabbott0]

Ok, that's strange... it must be that manually doing it reorders the adds/multiplies in such a way that makes us not hit that pathological case in the scheduler. Just for reference, you're now getting the same error as me when you don't hand-code it, right?

Assertion failed: (0), function try_schedule_reg, file gp_ir/scheduler.c, line 967.

[ssvb]

Just for reference, you're now getting the same error as me when you don't hand-code it, right?
Assertion failed: (0), function try_schedule_reg, file gp_ir/scheduler.c, line 967.

limasc: gp_ir/scheduler.c:950: try_schedule_reg: Assertion `0' failed.
Aborted