Thanks for the review! @ftynse

Perform dyn_cast and upon failure, return failure().

Avoid performing cast here but instead, use getMemorySpaceAsInt() API.

Thanks @ftynse ! Indeed, I didn't realize the review e-mail doesn't cover all the comments. I also managed to spectacularly misunderstand your comments on the sigils being 'outside' the pattern match. Hopefully, this time catches it all.

Much appreciated the detail review comments from @ftynse ! I went over each of them carefully and hopefully not missing anything. Please let me know if further improvements are needed! Thank you once again.

This patch catches the corner case where RHS is non-scalable vector and LHS is scalable vector, throw an error. When RHS is scalable vector, but LHS is non-scalable vector, this case is supported and testcase shows it. This patch looks good to me.

In D138980#3959192, @nicolasvasilache wrote:

Nice red diff :)

Thanks!

Thanks! Didn't realize this should also be updated!

@ftynse @chelini Thanks for the reviews. Would you be able to accept this revision? Much appreciated!

Clang-format causing Debian build to fail. Addressed in this patch.

All comments are addressed. Much appreciated! Please let me know if there are further comments.

In D133233#3789860, @Groverkss wrote:

The patch was landed. You can use Hermite decomp to do inverse here.

Thanks @Groverkss ! Any performance numbers to share?

[ RUN      ] MatrixTest.determinant
[       OK ] MatrixTest.determinant (106 ms)
[ RUN      ] MatrixTest.transpose
[       OK ] MatrixTest.transpose (0 ms)
[ RUN      ] MatrixTest.cofactor
[       OK ] MatrixTest.cofactor (107 ms)
[ RUN      ] MatrixTest.inverse
[       OK ] MatrixTest.inverse (111 ms)
[ RUN      ] MatrixTest.toVector
[       OK ] MatrixTest.toVector (0 ms)
[ RUN      ] MatrixTest.Matrix
[       OK ] MatrixTest.Matrix (108 ms)
[----------] 13 tests from MatrixTest (432 ms total)

A simple reason I can see why the computation is expensive is that the determinant algorithm written is exponential, and each call makes a copy instead of creating a view. Fixing this would improve the performance quite a bit, but I would wait for the next round of review. Also, it would be helpful to know why you don't use LU decomposition (or Hermite normal form) before I can review the algorithm.

@Groverkss Thanks for the detailed review. gtest does show the algorithm is tending on the expensive side. There was a reason that we went the route of computing the determinant/cofactor as suppose to a method such as LU decomposition. I can find out the precise reason from our scheduler person and update the MR tomorrow. Hope the style better matches your expectation and thanks for catching the zero-row vector bug.

In D133236#3769784, @ldrumm wrote:

Do you have commit access?