Inefficient array code generation

[Rüdiger Franke]

See the following example that just was added to the testsuite:

model AliasArrayTest
  Real[3,3] A = {{t, 2, 3}, {4, t, 6}, {7, 8, t}}; // RefArrayDim2
  Real[:] b = (t + 1) * {1, 2, 3};
  Real t = time;
  output Real[3] x;
equation
  x = Modelica.Math.Matrices.solve(A, b);
  annotation(experiment(StopTime = 0));
end AliasArrayTest;

OpenModelica generates very inefficient simcode:

x[1] = Modelica.Math.Matrices.solve({{A[3,3], 2.0, 3.0}, {4.0, A[3,3], 6.0}, {7.0, 8.0, A[3,3]}}, {b[1], b[2], b[3]})[1];
x[2] = Modelica.Math.Matrices.solve({{A[3,3], 2.0, 3.0}, {4.0, A[3,3], 6.0}, {7.0, 8.0, A[3,3]}}, {b[1], b[2], b[3]})[2];
x[3] = Modelica.Math.Matrices.solve({{A[3,3], 2.0, 3.0}, {4.0, A[3,3], 6.0}, {7.0, 8.0, A[3,3]}}, {b[1], b[2], b[3]})[3];

Meaning the equation system is solved once for each element of the solution vector and two temporary arrays are created for each call to solve.

The generated simcode should read:

x = Modelica.Math.Matrices.solve(A, b);

A similar thing was solved recently for tuples, see ​BackendDAE.ASSIGN can hold tuples on lhs.

This example can be simplified manually in the generated Cpp code (C as well I think). Then also the storage order of RefArray plays a role, see ​Unify storage order of RefArray.

[Rüdiger Franke]

The generated simcode improves if the called function has more than one return argument:

  x := Modelica.Math.Matrices.LAPACK.dgesv_vec(A, b);

gives

  (x, _) := Modelica.Math.Matrices.LAPACK.dgesv_vec({{A[3,3], 2.0, 3.0}, {4.0, A[3,3], 6.0}, {7.0, 8.0, A[3,3]}}, {b[1], b[2], b[3]});

The function is only called once now. The unnecessary temporary arrays remain though.

[Martin Sjölund]

Milestone moved to 1.12.0 due to 1.11.0 already being released.

[Martin Sjölund]

With #4453, the generated call is sane. Still got the temporary arrays due to evaluation though. I am wondering if we could perhaps detect this case when collapsing array references. Would need to pass along the aliases and known variables though (to verify A[3,3] is allowed when A[1,1] is expected when collapsing the arrays)...

[Martin Sjölund]

Looking through the generated code... The problem is that everything is aliases and thus not really part of the regular arrays which means we can't point somewhere in memory. So collapsing the array would mean we have to undo removeSimpleEquations to get things back. There are similar issues with code generation such as arrays in records with multiple fields, where you end up with a stride of for example 3.

We could rewrite the array handling at runtime to not just take a pointer to A[1,1] and a size of a number of elements, but instead we say that an array is a pointer to for example data->localData[0]->realVars, the size of the arrays and the offsets (causing indirection for each lookup). The offsets would be constant memory though (the constants in the array would need to be dummy variables in the realVars array). Or some C++-style polymorphism.

The main problem with these temporary arrays is that they use the memory allocator. We wouldn't really want to create some global/static temporary array due to the desire for functions to be reentrant. We could make the array allocated on the heap and just copy elements (slow if there are many constants in there; OK if we copy alias vars).

We could keep space for alias and known variables during run-time in the realVars array. Then we would only need to copy the alias variables before constructing the array (not at the output step, but in each step of the ODE function being called, etc). This costs some additional memory and copying during run-time. (This would not fix the problem with array slices and a bit odd array accesses causing non-1 strides)

We could also not remove known/alias variables in arrays that are referenced in whole (might also require --vectorizationLimit=1 so the frontend keeps these references).

Does anyone have any thoughts on the best way to proceed with this?

[Francesco Casella]

Milestone moved to 1.13.0 due to 1.12.0 already being released.

[Francesco Casella]

Rescheduled to 1.14.0 after 1.13.0 releasee

[Francesco Casella]

Releasing 1.14.0 which is stable and has many improvements w.r.t. 1.13.2. This issue is rescheduled to 1.16.0

[Francesco Casella]

Retargeted to 1.17.0 after 1.16.0 release

[Francesco Casella]

Retargeted to 1.18.0 because of 1.17.0 timed release.

[Francesco Casella]

Ticket retargeted after milestone closed