Skip to content

Update IMEX/LLVM versions #399

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 29, 2024
Merged

Update IMEX/LLVM versions #399

merged 4 commits into from
Oct 29, 2024

Conversation

dchigarev
Copy link
Contributor

@dchigarev dchigarev commented Oct 28, 2024

Fixes #392

This PR adapts GC to the new version on IMEX&LLVM.

The "new version of imex" is the mlir-extension/gc-staging branch that was rebased onto mlir-extensions/main. The rebased gc-staging branch lives in my own fork for now.

How I'm planing to merge this:

  1. Get this PR (Update IMEX/LLVM versions #399) approved.
  2. Force-push dchigarev/mlir-extensions/gc-staging-2 branch into mlir-extensions/gc-staging
    ----- CI in GC is broken at this point, since it'll try to fetch commits from gc-staging that no longer exist -----
  3. Change imex commit in cmake/imex-version.txt in this PR (Update IMEX/LLVM versions #399) to point to the new head of mlir-extensions/gc-staging.
  4. Merge this (Update IMEX/LLVM versions #399) PR into GC

If someone has a better suggestion on how to do it better, I'll be glad to hear it

Signed-off-by: dchigarev <dmitry.chigarev@intel.com>
Signed-off-by: dchigarev <dmitry.chigarev@intel.com>
Comment on lines -767 to +769
packedAttr = mlir::UnitAttr::get(rewriter.getContext());
if (!transpose_bit) {
packedAttr = mlir::UnitAttr::get(rewriter.getContext());
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Specifying both transpose_bit and packed in xegpu.load_nd is no longer supported. If transpose_bit is specified then the op already behaves as if packed was set.

Comment on lines -1168 to 1170
VnniConfig vnniConfA{.vnniFactor = vnniFactor, .vnniAxis = 1};
VnniConfig vnniConfB{.vnniFactor = vnniFactor, .vnniAxis = 0};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

xegpu.dpas no longer supports A arguments to be a 3D vector

Comment on lines -1632 to +1634
LinalgToXeGPUOptions options{kTile, stages, dpasTile};
LinalgToXeGPUOptions options{
kTile, stages, SmallVector<int64_t>(dpasTile.begin(), dpasTile.end())};
Copy link
Contributor Author

@dchigarev dchigarev Oct 29, 2024

Choose a reason for hiding this comment

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

Does the conversion explicitly

error: could not convert ‘dpasTile’ from ‘mlir::Pass::ListOption<long int>’ to ‘llvm::SmallVector<long int>’

@dchigarev dchigarev marked this pull request as ready for review October 29, 2024 12:45
Copy link
Contributor

@kurapov-peter kurapov-peter left a comment

Choose a reason for hiding this comment

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

Sounds good to me. I'd like to detach linalg-to-xegpu from imex. This seems to be a good time to do that since we'll have the same hash for llvm.

Signed-off-by: dchigarev <dmitry.chigarev@intel.com>
@dchigarev dchigarev merged commit 6feea7f into main Oct 29, 2024
6 checks passed
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.

Update IMEX version
3 participants