Skip to content

Conversation

@arun-thmn
Copy link
Contributor

@arun-thmn arun-thmn commented Jul 14, 2025

Fix after this bump:

  • Tune related code got removed as we use upstream MLIR code
  • Syntax update to ConstantFloatOp and arith::ConstantIntOp creation
  • ArrayRef is deprecated, so update with equivalent syntax
  • linalg::vectorizer w.r.t PR:#144158
  • linalg::pack w.r.t PR:#146139

@arun-thmn arun-thmn marked this pull request as ready for review July 14, 2025 15:36
@arun-thmn
Copy link
Contributor Author

Hey @adam-smnk,
Needed your help/guidance in this LLVM bump PR. There is an update to the upstream linalg::vectorizer. Both the earlier vectorization pass and #1060 linalg-vectorizer affected.

@arun-thmn arun-thmn requested a review from adam-smnk July 14, 2025 15:42
@arun-thmn
Copy link
Contributor Author

arun-thmn commented Jul 15, 2025

@adam-smnk Now, the upstream vectorized pass returns values instead of replacing [https://github.com/llvm/llvm-project/pull/144158]. So, the replacing of linalop has to be done in our passes. So, kind of fixed the problem.

Presently, trying to sort an issue with packs.

@arun-thmn
Copy link
Contributor Author

arun-thmn commented Jul 15, 2025

@adam-smnk After bump LLVM, the pack is not working for few tests. One example is:

// CHECK: %[[PACK3:.+]] = linalg.pack %[[ARG2]] outer_dims_perm = [0, 3, 1, 2] inner_dims_pos = [3] inner_tiles = [32] into %[[BUFF3]] : tensor<1x56x56x64xf32> -> tensor<1x2x56x56x32xf32>

The pack works bump IR is:
%4 = tensor.empty() : tensor<1x2x56x56x32xf32>
%pack_2 = linalg.pack %arg2 outer_dims_perm = [0, 3, 1, 2] inner_dims_pos = [3] inner_tiles = [32] into %4 : tensor<1x56x56x64xf32> -> tensor<1x2x56x56x32xf32>
%5 = tensor.empty() : tensor<2x32xf32>
%pack_3 = linalg.pack %arg3 outer_dims_perm = [0] inner_dims_pos = [0] inner_tiles = [32] into %5 : tensor<64xf32> -> tensor<2x32xf32>
%6 = linalg.generic {indexing_maps = [#map3, #map4, #map3], iterator_types = ["parallel", "parallel", "parallel", "parallel", "parallel"]} ins(%3, %pack_3 : tensor<1x2x56x56x32xf32>, tensor<2x32xf32>) outs(%pack_2 : tensor<1x2x56x56x32xf32>) {

After the bump IR:
%4 = tensor.empty() : tensor<1x2x56x56x32xf32>
%5 = tensor.empty() : tensor<2x32xf32>
%pack_2 = linalg.pack %arg3 outer_dims_perm = [0] inner_dims_pos = [0] inner_tiles = [32] into %5 : tensor<64xf32> -> tensor<2x32xf32>
%6 = linalg.generic {indexing_maps = [#map3, #map4, #map3], iterator_types = ["parallel", "parallel", "parallel", "parallel", "parallel"]} ins(%3, %pack_2 : tensor<1x2x56x56x32xf32>, tensor<2x32xf32>) outs(%4 : tensor<1x2x56x56x32xf32>) {

It might invest some time to fins out the root-cause. If you identify the problem handy (as you know the pass details), let me know please. Else, I will try to find it out.

@adam-smnk
Copy link
Contributor

adam-smnk commented Jul 15, 2025

@arun-thmn That's probably due to recent changes in one of propagation/packing patterns.
I think this one llvm/llvm-project#146139. When %out is unused in generic's body, outs argument is no longer packed but a reshaped tensor.empty is applied directly (or sth like that).

It should be fine, I had a look at the changes and they seem correct. Just please double check the results are still correct in our test case - since it's not an integration test, just follow data manually.

@arun-thmn
Copy link
Contributor Author

@arun-thmn That's probably due to recent changes in one of propagation/packing patterns. I think this one llvm/llvm-project#146139. When %out is unused in generic's body, outs argument is no longer packed but a reshaped tensor.empty is applied directly (or sth like that).

It should be fine, I had a look at the changes and they seem correct. Just please double check the results are still correct in our test case - since it's not an integration test, just follow data manually.

Thanks @adam-smnk, I will remove the pack from our unit-test case and will check the results.

@arun-thmn arun-thmn changed the title [wip]: Bump llvm Bump llvm Jul 15, 2025
@arun-thmn arun-thmn requested review from rengolin and rolfmorel July 15, 2025 08:38
@arun-thmn
Copy link
Contributor Author

@adam-smnk double checked the results of pack unit-tests with tpp-run, they are good.

Copy link
Contributor

@adam-smnk adam-smnk left a comment

Choose a reason for hiding this comment

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

LGTM % python/transform changes

@adam-smnk
Copy link
Contributor

Could you also run benchmarks just to see if there are any failures there?

@arun-thmn arun-thmn added the benchmark-full Benchmark all targets label Jul 15, 2025
@arun-thmn
Copy link
Contributor Author

Could you also run benchmarks just to see if there are any failures there?

Ran benchmarks-all no failures.

@arun-thmn arun-thmn merged commit b4efad4 into libxsmm:main Jul 15, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

benchmark-full Benchmark all targets

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants