Multiplication operand order

[mario-deluna]

Description

In reference to #3735, I'm struggling with a very similar problem. I'm working on a GL math extension and have encountered a roadblock with matrix multiplication.

As @weltling pointed out, the order is critical, and I can see that this commit 1165a90 removed ZEND_MUL from one of the optimizer passes. I'm not 100% certain the swapping occurs in the optimizer as I ran into the swapped arguments also with the opcache turned off and opcache.optimization_level=0x00000000.

The extension utilizes the do_operation handler. Here is example code to explain myself better.

The problem only occurs when multiplying three or more objects in one expression.

<?php 
use GL\Math\Mat4;

$m1 = new Mat4(
  1,  2,  3,  4,
  5,  6,  7,  8,
  9,  10, 11, 12,
  13, 14, 15, 16
);
$m2 = new Mat4(
  16, 15, 14, 13, 
  12, 11, 10,  9, 
   8,  7,  6,  5, 
   4,  3,  2,  1
);
$m3 = new Mat4(
  0.16, 0.15, 0.14, 0.13, 
  0.12, 0.11, 0.10, 0.90, 
  0.80, 0.70, 0.60, 0.50, 
  0.40, 0.30, 0.20, 0.10
);


// works fine, correct order
var_dump($m1 * $m2); 

// incorrect result, order of execution is:
// logged: MUL(tmp_res, $m1 * $m2), MUL(returnval, $m3 * tmp_res)
// expected: MUL(tmp_res, $m1 * $m2), MUL(returnval, tmp_res * $m3)
var_dump($m1 * $m2 * $m3); 

// you can workaround using parenthesis
// logged: MUL(tmp_res, $m2 * $m3), MUL(returnval, $m1 * tmp_res)
var_dump($m1 * ($m2 * $m3)); 

// but for the expected result it should not matter where parenthesis are set and the order should be fixed.

I've tried to look for workarounds by somehow detecting when and if they have been swapped but have had no luck yet.

I gladly open a pull request if this requires some work in php-src itself, but I would kindly ask for some hints and direction on where this might happen.

// the operation handler implementation (striped down)
static int math_mat4_do_op_handler(zend_uchar opcode, zval *result, zval *op1, zval *op2)
{
    object_init_ex(result, math_mat4_ce);
    math_mat4_object *resobj = math_mat4_objectptr_from_zobj_p(Z_OBJ_P(result));

    if (
        Z_TYPE_P(op1) == IS_OBJECT && Z_OBJCE_P(op1) == math_mat4_ce &&
        Z_TYPE_P(op2) == IS_OBJECT && Z_OBJCE_P(op2) == math_mat4_ce    ) 
    {
        math_mat4_object *matobj1 = math_mat4_objectptr_from_zobj_p(Z_OBJ_P(op1));
        math_mat4_object *matobj2 = math_mat4_objectptr_from_zobj_p(Z_OBJ_P(op2));

        switch (opcode) {
        case ZEND_MUL:
            mat4x4_mul(resobj->data, matobj1->data, matobj2->data);
            return SUCCESS;
        default:
            return FAILURE;
        }
    }
    else {
        return FAILURE;
    }
}

Kind regards Mario

PHP Version

PHP 8.1.5

Operating System

MacOS 12.4

[cmb69]

Oh, apparently that fix didn't last long, since 588f1df broke it. Would probably have been a good idea to add a test.

[Girgias]

Do you know a version that does work? And the best would be if you could git bisect to the breaking commit (or atleast the patch version that breaks) as I do have a test but as it's incorporated into zend_test I currently only have it for 8.2 (which maybe was my mistake but)

[cmb69]

And the best would be if you could git bisect to the breaking commit

588f1df broke it.

[Girgias]

So it's been broken for 5 years O.o I thought because Nikita passed through the optimizer recently it wasn't broken since forever