This is an archive of the discontinued LLVM Phabricator instance.

[mlir][Transform][LinAlg] Add copy permutation for GPU memory hierarchy in the `transform.promote` op
AcceptedPublic

Authored by tavakkoliamirmohammad on Feb 28 2023, 10:44 AM.

Details

Summary

In this patch, we are adding a copy permutation to the transform.promote op. This option is available when using the new mapping attribute to the transform.promote op. This allows changing the logical view of the data (memref.subview) using memref.transpose without moving the data.

If the copy permutation is given by the user, the result operation will be the generated operation linalg.generic otherwise the original linalg target operation.

Diff Detail

Event Timeline

tavakkoliamirmohammad requested review of this revision.Feb 28 2023, 10:44 AM

Fix merge conflict

ftynse added inline comments.Mar 1 2023, 12:05 PM
mlir/lib/Dialect/Linalg/Transforms/Promotion.cpp
87–88

isa followed by cast (let alone dyn_cast) is an anti-pattern. dyn_cast itself does the type check and returns null on failure, so something like if (auto subview = dyn_cast<memref::SubViewOp>(op)) should be used instead. If you want to be fancy, you can do

auto viewType = llvm::TypeSwitch<ShapedType>(op)
  .Case<SubViewOp, TransposeOp>([](auto casted) { return casted.getType(); })
  .Default([](Operation *) { return nullptr; });
if (!viewType) return std::nullopt;
mlir/test/Dialect/Linalg/promote.mlir
287

CHECK-LABEL

299

Do we really need to pattern-match exact types here? We strive to have minimal tests that exercise the relevant behavior.

327

Please add the newline.

tavakkoliamirmohammad marked 4 inline comments as done.Mar 1 2023, 1:09 PM
tavakkoliamirmohammad added inline comments.
mlir/lib/Dialect/Linalg/Transforms/Promotion.cpp
87–88

Thank you for suggesting this elegant way!

tavakkoliamirmohammad marked an inline comment as done.

Update the diff based on the comment

ftynse added a comment.Mar 1 2023, 5:28 PM

LGTM when the last comment is addressed, but I'd like an opinion from @nicolasvasilache on the promotion logic.

mlir/lib/Dialect/Linalg/Transforms/Promotion.cpp
478–484

Same as the code above, isa followed by dyn_cast is an anti-pattern.

tavakkoliamirmohammad marked an inline comment as done.

Fixed anti-pattern and return condition of allocateSubviewGPUMemoryInAddressSpace function

Hi @nicolasvasilache, just wanted to check in and see if you had a chance to review the promotion logic yet. If you have any thoughts or feedback, it would be great to hear them. Thanks!

ftynse accepted this revision.Mar 14 2023, 11:26 AM
This revision is now accepted and ready to land.Mar 14 2023, 11:26 AM

Thank you! I don't have the commit access to LLVM project for applying this patch.

Could you please rebase?