Skip to content

[Integrate] Drop LLVM revert of "Remove matmul_transpose variants"#21344

Merged
Yu-Zhewen merged 13 commits into
mainfrom
integrates/llvm-cherry-pick-147961
Aug 16, 2025
Merged

[Integrate] Drop LLVM revert of "Remove matmul_transpose variants"#21344
Yu-Zhewen merged 13 commits into
mainfrom
integrates/llvm-cherry-pick-147961

Conversation

@hanhanW

@hanhanW hanhanW commented Jul 11, 2025

Copy link
Copy Markdown
Contributor

Issue: #21349

@rengolin

Copy link
Copy Markdown

Awesome @hanhanW, thanks!

All errors seem to be about this particular problem:

external/llvm-project/llvm/include/llvm/Support/Casting.h:64:65: error: cannot initialize a parameter of type 'mlir::Operation *' with an rvalue of type 'const mlir::linalg::LinalgOp *'
  static inline bool doit(const From &Val) { return To::classof(&Val); }
                                                                ^~~~
external/llvm-project/llvm/include/llvm/Support/Casting.h:81:32: note: in instantiation of member function 'llvm::isa_impl<mlir::linalg::BatchMatmulTransposeBOp, mlir::linalg::LinalgOp>::doit' requested here
    return isa_impl<To, From>::doit(Val);
                               ^
external/llvm-project/llvm/include/llvm/Support/Casting.h:137:37: note: in instantiation of member function 'llvm::isa_impl_cl<mlir::linalg::BatchMatmulTransposeBOp, const mlir::linalg::LinalgOp>::doit' requested here
    return isa_impl_cl<To, FromTy>::doit(Val);
                                    ^
external/llvm-project/llvm/include/llvm/Support/Casting.h:257:58: note: in instantiation of member function 'llvm::isa_impl_wrap<mlir::linalg::BatchMatmulTransposeBOp, const mlir::linalg::LinalgOp, const mlir::linalg::LinalgOp>::doit' requested here
        typename simplify_type<const From>::SimpleType>::doit(f);
                                                         ^
external/llvm-project/llvm/include/llvm/Support/Casting.h:549:36: note: in instantiation of member function 'llvm::CastIsPossible<mlir::linalg::BatchMatmulTransposeBOp, const mlir::linalg::LinalgOp>::isPossible' requested here
  return CastInfo<To, const From>::isPossible(Val);
                                   ^
external/llvm-project/llvm/include/llvm/Support/Casting.h:554:29: note: in instantiation of function template specialization 'llvm::isa<mlir::linalg::BatchMatmulTransposeBOp, mlir::linalg::LinalgOp>' requested here
  return isa<First>(Val) || isa<Second, Rest...>(Val);
                            ^
external/llvm-project/llvm/include/llvm/Support/Casting.h:554:29: note: in instantiation of function template specialization 'llvm::isa<mlir::linalg::BatchMatmulOp, mlir::linalg::BatchMatmulTransposeBOp, mlir::linalg::LinalgOp>' requested here
external/llvm-project/llvm/include/llvm/Support/Casting.h:554:29: note: in instantiation of function template specialization 'llvm::isa<mlir::linalg::MatmulTransposeBOp, mlir::linalg::BatchMatmulOp, mlir::linalg::BatchMatmulTransposeBOp, mlir::linalg::LinalgOp>' requested here
external/llvm-project/llvm/include/llvm/Support/Casting.h:678:10: note: in instantiation of function template specialization 'llvm::isa<mlir::linalg::MatmulOp, mlir::linalg::MatmulTransposeBOp, mlir::linalg::BatchMatmulOp, mlir::linalg::BatchMatmulTransposeBOp, mlir::linalg::LinalgOp>' requested here
  return isa<X...>(Val);
         ^
external/llvm-project/llvm/include/llvm/Support/Casting.h:683:10: note: in instantiation of function template specialization 'llvm::isa_and_present<mlir::linalg::MatmulOp, mlir::linalg::MatmulTransposeBOp, mlir::linalg::BatchMatmulOp, mlir::linalg::BatchMatmulTransposeBOp, mlir::linalg::LinalgOp>' requested here
  return isa_and_present<X...>(Val);
         ^
compiler/src/iree/compiler/Preprocessing/Common/GeneralizeLinalgMatMul.cpp:33:11: note: in instantiation of function template specialization 'llvm::isa_and_nonnull<mlir::linalg::MatmulOp, mlir::linalg::MatmulTransposeBOp, mlir::linalg::BatchMatmulOp, mlir::linalg::BatchMatmulTransposeBOp, mlir::linalg::LinalgOp>' requested here
      if (isa_and_nonnull<linalg::MatmulOp, linalg::MatmulTransposeBOp,
          ^
external/llvm-project/mlir/include/mlir/Dialect/Linalg/IR/Linalg.h:264:34: note: passing argument to parameter 'op' here
  static bool classof(Operation *op);
                                 ^
2 errors generated.

I have fixed an issue with the classof yesterday and this is the commit that you do not seem to have it here.

Can you cherry pick again on the latest commit of that PR and see if the issues go away?

@rengolin

Copy link
Copy Markdown

@kuhar this is what you meant on the llvm pr, I guess.

@hanhanW

hanhanW commented Jul 17, 2025

Copy link
Copy Markdown
Contributor Author

Thanks! I'll also rebase to the main branch, which has fresh LLVM commits.

@hanhanW hanhanW force-pushed the integrates/llvm-cherry-pick-147961 branch from d68c020 to 32640f8 Compare July 17, 2025 22:58
@rengolin

Copy link
Copy Markdown

I have rebase my pr on a recent branch, so should be easy to cherry pick.

@hanhanW hanhanW closed this Jul 21, 2025
@hanhanW hanhanW force-pushed the integrates/llvm-cherry-pick-147961 branch from 32640f8 to 28c2301 Compare July 21, 2025 18:15
@hanhanW hanhanW reopened this Jul 21, 2025
@Abhishek-Varma Abhishek-Varma requested a review from Yu-Zhewen July 22, 2025 08:34
@Abhishek-Varma Abhishek-Varma force-pushed the integrates/llvm-cherry-pick-147961 branch 3 times, most recently from 7120f2a to 42d2e6e Compare July 22, 2025 09:39
@Yu-Zhewen Yu-Zhewen force-pushed the integrates/llvm-cherry-pick-147961 branch 5 times, most recently from 6aa0f5f to 061c693 Compare July 22, 2025 20:07
@hanhanW hanhanW force-pushed the integrates/llvm-cherry-pick-147961 branch 2 times, most recently from 538cad9 to ef13595 Compare July 23, 2025 17:06
@Yu-Zhewen Yu-Zhewen force-pushed the integrates/llvm-cherry-pick-147961 branch from ef13595 to a825464 Compare July 23, 2025 19:34
@Yu-Zhewen Yu-Zhewen force-pushed the integrates/llvm-cherry-pick-147961 branch from a825464 to 8e3828a Compare July 23, 2025 19:36
@Yu-Zhewen Yu-Zhewen force-pushed the integrates/llvm-cherry-pick-147961 branch from 20b06b2 to b060031 Compare August 14, 2025 17:24
@Yu-Zhewen Yu-Zhewen changed the title DNS - Cherry-pick llvm "Remove matmul_transpose variants" [Integrate] Drop LLVM revert of "Remove matmul_transpose variants" Aug 14, 2025
@kuhar

kuhar commented Aug 15, 2025

Copy link
Copy Markdown
Member

This needs to be rebased

Yu-Zhewen and others added 12 commits August 15, 2025 16:31
…#147961)""

Signed-off-by: Yu-Zhewen <zhewenyu@amd.com>
Signed-off-by: Abhishek Varma <abhvarma@amd.com>
Signed-off-by: Yu-Zhewen <zhewenyu@amd.com>
Signed-off-by: Yu-Zhewen <zhewenyu@amd.com>
Signed-off-by: Yu-Zhewen <zhewenyu@amd.com>
Signed-off-by: Yu-Zhewen <zhewenyu@amd.com>
Signed-off-by: Yu-Zhewen <zhewenyu@amd.com>
Signed-off-by: Yu-Zhewen <zhewenyu@amd.com>
Signed-off-by: Yu-Zhewen <zhewenyu@amd.com>
Signed-off-by: Yu-Zhewen <zhewenyu@amd.com>
…71716e88bd4af8f22c

Signed-off-by: Yu-Zhewen <zhewenyu@amd.com>
Signed-off-by: Yu-Zhewen <zhewenyu@amd.com>
@Yu-Zhewen Yu-Zhewen force-pushed the integrates/llvm-cherry-pick-147961 branch from b060031 to efe5104 Compare August 15, 2025 16:32
@Yu-Zhewen

Copy link
Copy Markdown
Contributor

This needs to be rebased

Done

Comment on lines +468 to +469
Operation *op = bmmOp.getOperation();
if (!IREE::LinalgExt::isPureBatchMatmul(op)) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can just be !IREE::LinalgExt::isPureBatchMatmul(bmmOp)/

Comment on lines +452 to +453
Operation *op = matmulOp.getOperation();
if (!IREE::LinalgExt::isPureMatmul(op)) {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Comment thread compiler/src/iree/compiler/Codegen/LLVMCPU/KernelDispatch.cpp
Comment thread compiler/src/iree/compiler/Codegen/LLVMCPU/KernelDispatch.cpp
Comment on lines +951 to +954
bool isPureMatmul(Operation *op) {
return dyn_cast_or_null<linalg::MatmulOp>(op) &&
linalg::MatmulOp::isDefaultIndexingMaps(op->getAttr("indexing_maps"));
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd do it this way, which avoids magic string.

auto matmulOp = dyn_cast_or_null<linalg::MatmulOp>(op);
return matmulOp && linalg::MatmulOp::isDefaultIndexingMaps(matmulOp.getIndexingMaps());

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we don't need to check maps for this file, because it is a test for matching. We should just check match_status.

Comment on lines +957 to +961
bool isPureBatchMatmul(Operation *op) {
return dyn_cast_or_null<linalg::BatchMatmulOp>(op) &&
linalg::BatchMatmulOp::isDefaultIndexingMaps(
op->getAttr("indexing_maps"));
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs to be updated as well. Sorry I forgot to point this out

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank. Done

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do it in a follow-up, because I really want to drop the revert first. Functionality-wise, it is okay.

The better implementation in my mind is that we bail out if the op is not a pure matmul/batch_matmul or with transpose variants.

Then we look at the indexing maps to create transpose op + matmul/batch_matmul ops.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I misread the code. never mind.

Signed-off-by: Yu-Zhewen <zhewenyu@amd.com>
@Yu-Zhewen Yu-Zhewen force-pushed the integrates/llvm-cherry-pick-147961 branch from 9e28846 to 6f33b05 Compare August 15, 2025 21:29
@hanhanW

hanhanW commented Aug 15, 2025

Copy link
Copy Markdown
Contributor Author

I just realized that I can't approve the PR because I created the PR. More eyes are better, @kuhar can you review and stamp?

@kuhar kuhar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GeneralizeNamedOps may also need to be updated: #21716

cc: @newling

@Yu-Zhewen Yu-Zhewen merged commit 3df06b7 into main Aug 16, 2025
48 checks passed
@Yu-Zhewen Yu-Zhewen deleted the integrates/llvm-cherry-pick-147961 branch August 16, 2025 08:35
AWoloszyn pushed a commit that referenced this pull request Dec 1, 2025
…21344)

Issue: #21349

---------

Signed-off-by: Yu-Zhewen <zhewenyu@amd.com>
Signed-off-by: Abhishek Varma <abhvarma@amd.com>
Co-authored-by: Yu-Zhewen <zhewenyu@amd.com>
Co-authored-by: Abhishek Varma <abhvarma@amd.com>
Co-authored-by: Ean Garvey <ean.garvey@amd.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants