[PATCH] D133657: [Matrix] Optimize matrix transposes around additions

[Francis Visoiu Mistrih via Phabricator via llvm-commits]

thegameg added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:832
+                [&](Value *T0, ShapeInfo Shape0, Value *T1, ShapeInfo Shape1) {
+                  auto *FAdd = cast<Instruction>(
+                      LocalBuilder.CreateFAdd(T0, T1, "mfadd"));
----------------
fhahn wrote:
> Could the lambda just return `Value *` instead of `Instruction *`? `setShapeInfo` and `ReplaceAllUsesWith` should work with `Value *`?
It could, but we would need to then downcast back to `Instruction` for the iterator update:

```
        // If we replaced I with a new instruction, continue from there.
        if (NewInst)
          II = std::next(BasicBlock::reverse_iterator(NewInst));
```


================
Comment at: llvm/lib/Transforms/Scalar/LowerMatrixIntrinsics.cpp:882
+                 match(B,
+                       m_Intrinsic<Intrinsic::matrix_transpose>(
+                           m_Value(BT), m_ConstantInt(R), m_ConstantInt(C)))) {
----------------
fhahn wrote:
> Should we introduce a new `m_Transpose` helper to make this a bit more compact?
Good idea, let me do it in a separate patch so it doesn't clutter the diff here.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D133657/new/

https://reviews.llvm.org/D133657