Skip to content

Fix LIT test issue - #8

Draft
bmahjour wants to merge 1 commit into
mainfrom
bmahjour/fix-build-and-test-issues
Draft

bmahjour wants to merge 1 commit into
mainfrom
bmahjour/fix-build-and-test-issues

Conversation

@bmahjour

@bmahjour bmahjour commented Apr 16, 2026

Copy link
Copy Markdown
Collaborator

This PR fixes a LIT test failure in Ktdp/construct-indirect-access-tile.mlir because it expect the ind(...) operands in the wrong order!

@bmahjour bmahjour self-assigned this Apr 16, 2026
@bmahjour
bmahjour marked this pull request as draft April 16, 2026 17:36
@fabianlim

Copy link
Copy Markdown
Collaborator

@bmahjour we are aware of this. The reason you are seeing these error is probably because you are building against an older version of LLVM, older than the v22 that we are using as a current default, see this issue

  • as such, our CI is currently building against v22, and as evident by the cmake-test failure, your proposed change will not work with v22.
  • please rest assured we are taking into account that the older version is the one currently in torch-spyre, but we are adopting a forward-lookin approach

If there is some reason you can afford to change version, then may I suggest you follow the build instructions provided in the README. We have multiple options including building with official llvm wheels

@bmahjour

Copy link
Copy Markdown
Collaborator Author

Yes, I noticed the difference in LLVM versions @fabianlim. I made one last attempt to see if we can resolve the dependencies indirectly through libs that work for both v20 and v22, but that doesn't seem to work either, so I'll revert all my cmake changes.

Please note that this PR also fixes one of the LIT tests. Isn't that test failing in master when built against LLVM v22?

@fabianlim

fabianlim commented Apr 16, 2026

Copy link
Copy Markdown
Collaborator

Please note that this PR also fixes one of the LIT tests. Isn't that test failing in master when built against LLVM v22?

Oh interesting. Do you know which lit test you are referring to? According to what I see here I have 17 tests and all are passing.

If you are referring to the test with the ordering issue in that CHECK, then I know that one is passing in v22

@fabianlim

Copy link
Copy Markdown
Collaborator

@bmahjour this are the tests that are being run and passing now on v22

@bmahjour

Copy link
Copy Markdown
Collaborator Author

@bmahjour this are the tests that are being run and passing now on v22@bmahjour this are the tests that are being run and passing now on v22

Interestingly construct-indirect-access-tile.mlir is passing even though it expects the add operands of ind() in reverse order from the input.

I tried building with LLVM v22.1 4434dabb69916856b824f68a64b029c67175e532 but I'm now getting a different error:

> ./bin/ktir-opt ../test/Ktdp/basic.mlir
LLVM ERROR: can't create Attribute 'mlir::DenseArrayAttr' because storage uniquer isn't initialized: the dialect was likely not loaded, or the attribute wasn't added with addAttributes<...>() in the Dialect::initialize() method.
PLEASE submit a bug report to https://github.com/llvm/llvm-project/issues/ and include the crash backtrace and instructions to reproduce the bug.
Stack dump:
0.	Program arguments: ./bin/ktir-opt ../test/Ktdp/basic.mlir
1.	MLIR Parser: custom op parser 'builtin.module'
2.	MLIR Parser: custom op parser 'func.func'
3.	MLIR Parser: custom op parser 'ktdp.construct_memory_view'
Stack dump without symbol names (ensure you have llvm-symbolizer in your PATH or set the environment var `LLVM_SYMBOLIZER_PATH` to point to it):
0  libLLVM.dylib            0x0000000122b62794 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) + 56
1  libLLVM.dylib            0x0000000122b5fb98 llvm::sys::RunSignalHandlers() + 172
2  libLLVM.dylib            0x0000000122b6327c SignalHandler(int, __siginfo*, void*) + 344
3  libsystem_platform.dylib 0x000000018b271764 _sigtramp + 56
4  libsystem_pthread.dylib  0x000000018b267888 pthread_kill + 296
5  libsystem_c.dylib        0x000000018b16c850 abort + 124
6  libLLVM.dylib            0x0000000122a89c9c llvm::report_fatal_error(llvm::Twine const&, bool) + 448
7  ktir-opt                 0x0000000105c808f8 bool llvm::function_ref<bool (mlir::StorageUniquer::BaseStorage const*)>::callback_fn<mlir::detail::DenseArrayAttrStorage* mlir::StorageUniquer::get<mlir::detail::DenseArrayAttrStorage, mlir::Type&, unsigned long, llvm::ArrayRef<char>&>(llvm::function_ref<void (mlir::detail::DenseArrayAttrStorage*)>, mlir::TypeID, mlir::Type&, unsigned long&&, llvm::ArrayRef<char>&)::'lambda'(mlir::StorageUniquer::BaseStorage const*)>(long, mlir::StorageUniquer::BaseStorage const*) + 0
8  ktir-opt                 0x0000000105c74cfc mlir::DenseArrayAttr mlir::detail::StorageUserBase<mlir::DenseArrayAttr, mlir::Attribute, mlir::detail::DenseArrayAttrStorage, mlir::detail::AttributeUniquer, mlir::BlobAttr::Trait>::get<mlir::Type&, unsigned long, llvm::ArrayRef<char>&>(mlir::MLIRContext*, mlir::Type&, unsigned long&&, llvm::ArrayRef<char>&) + 184
9  ktir-opt                 0x0000000105c758ec mlir::detail::DenseArrayAttrImpl<long long>::get(mlir::MLIRContext*, llvm::ArrayRef<long long>) + 76
10 libMLIR.dylib            0x000000011b3e0080 mlir::parseDynamicIndexList(mlir::OpAsmParser&, llvm::SmallVectorImpl<mlir::OpAsmParser::UnresolvedOperand>&, mlir::detail::DenseArrayAttrImpl<long long>&, mlir::detail::DenseArrayAttrImpl<bool>&, llvm::SmallVectorImpl<mlir::Type>*, mlir::AsmParser::Delimiter) + 188
11 ktir-opt                 0x00000001057bc8b8 mlir::parseDynamicIndexList(mlir::OpAsmParser&, llvm::SmallVectorImpl<mlir::OpAsmParser::UnresolvedOperand>&, mlir::detail::DenseArrayAttrImpl<long long>&, llvm::SmallVectorImpl<mlir::Type>*, mlir::AsmParser::Delimiter) + 72
12 ktir-opt                 0x00000001057bbf24 mlir::ktdp::ConstructMemoryViewOp::parse(mlir::OpAsmParser&, mlir::OperationState&) + 940
13 libMLIR.dylib            0x0000000116e5cc7c (anonymous namespace)::OperationParser::parseOperation() + 1184

Could you be more specific about the version of llvm you are building (ie what commit level)?

@fabianlim

fabianlim commented Apr 17, 2026

Copy link
Copy Markdown
Collaborator

Could you be more specific about the version of llvm you are building (ie what commit level)?

@bmahjour I am using the official llvm wheels, as seen from the uv.lock the short hash is 3b3ac5a11; the full hash is

$  curl -s "https://api.github.com/repos/llvm/llvm-project/commits?sha=3b3ac5
a1&per_page=1" \
>     | jq -r '.[0].sha'

3b3ac5a1169722bff1ae0f5f8f27a48cc08c3d02

Interestingly construct-indirect-access-tile.mlir is passing even though it expects the add operands of ind() in reverse order from the input.

This is from Claude

● Yes, this is a known MLIR change. Around LLVM 19→20, the op attribute dictionary printing switched to alphabetical order.
   Before that, attributes were printed in their ODS definition order. Since variables_space_order < variables_space_set
  alphabetically, newer LLVM prints order first — which is what the current CHECKs expect. On v19 they'd come out in
  ODS/parse order, which in the source is set then order.

● Found it. The custom printer at line 850 calls:
  p.printOptionalAttrDict(op->getAttrs(), ...)

  In older MLIR (v19 and earlier), op->getAttrs() returned attributes in insertion order. The build method adds
  variables_space_set first, then variables_space_order (lines 499-502 of KtdpOps.cpp), so v19 prints set before order.

  In newer MLIR (~v20+), op->getAttrs() returns attributes alphabetically sorted, so order comes before set — which matches
   the current CHECKs.

  The fix to make the test robust across both versions: swap the addAttribute calls in the build method to add
  variables_space_order before variables_space_set. That way insertion order matches alphabetical order, and the output is
  consistent on both old and new MLIR:

lib/Ktdp/KtdpOps.cpp
 496                        per_dim_subscript_kinds);
 497    result.addAttribute(getPerDimSubscriptMapsAttrStrName(),
 498                        per_dim_subscript_maps);
 499 -  result.addAttribute(getVariablesSpaceSetAttrStrName(),
 500 -                      IntegerSetAttr::get(variables_space_set));
 499 +  // Add in alphabetical order so insertion order matches the sorted order
 500 +  // used by newer MLIR (post-v19) when printing op->getAttrs(). This keeps
 501 +  // the printed output consistent across MLIR versions.
 502    result.addAttribute(getVariablesSpaceOrderAttrStrName(),
 503                        AffineMapAttr::get(variables_space_order));
 504 +  result.addAttribute(getVariablesSpaceSetAttrStrName(),
 505 +                      IntegerSetAttr::get(variables_space_set));
 5

This is what I get when I ran the problematic command.

(.venv) $  build/bin/ktir-opt test/Ktdp/basic.mlir 
#map = affine_map<(d0, d1) -> (d0, d1)>
#set = affine_set<(d0, d1) : (d0 >= 0, -d0 + 31 >= 0, d1 >= 0, -d1 + 63 >= 0)>
module {
  func.func @basic() {
    %c0 = arith.constant 0 : index
    %c1024 = arith.constant 1024 : index
    %0 = ktdp.construct_memory_view %c1024, sizes: [32, 64], strides: [64, 1] {coordinate_set = #set, memory_space = #ktdp.spyre_memory_space<HBM>} : memref<32x64xf16>
    %1 = ktdp.construct_distributed_memory_view(%0, %0 : memref<32x64xf16>, memref<32x64xf16>) : memref<64x64xf16>
    %2 = ktdp.construct_access_tile %0[%c0, %c0] {access_tile_order = #map, access_tile_set = #set} : memref<32x64xf16> -> !ktdp.access_tile<32x64xindex>
    %3 = ktdp.load %2 : <32x64xindex> -> tensor<32x64xf16>
    ktdp.store %3, %2 : tensor<32x64xf16>, <32x64xindex>
    return
  }
}

The commit you are using is from Feb. Actually now looking at it from

  • My commit 3b3ac5a1: April 9, 2026
  • Your commit 4434dabb: February 24, 2026

Looking at the release tags, I think yours is the official release, mine is a post-release commit on the main branch

  ⎿  llvmorg-23-init bd6bfba3e503
     llvmorg-22.1.3 e9846648fd61
     llvmorg-22.1.2 1ab49a973e21
     llvmorg-22.1.1 fef02d48c08d
     llvmorg-22.1.0 4434dabb6991

@fabianlim

fabianlim commented Apr 18, 2026

Copy link
Copy Markdown
Collaborator

@bmahjour just to follow up on your previous comment

I tried building with LLVM v22.1 4434dabb69916856b824f68a64b029c67175e532 but I'm now getting a different error:

I triggered a build on the above commit

  • the LLVM build is here
  • the KTIR build downloads the 4434dabb build here
  • the tests all pass

I will try running your command next ./bin/ktir-opt ../test/Ktdp/basic.mlir and checking if I get your error

Update: hmm yes it seems to work for me

build/bin/ktir-opt test/Ktdp/basic.mlir
$ build/bin/ktir-opt test/Ktdp/basic.mlir
#map = affine_map<(d0, d1) -> (d0, d1)>
#set = affine_set<(d0, d1) : (d0 >= 0, -d0 + 31 >= 0, d1 >= 0, -d1 + 63 >= 0)>
module {
  func.func @basic() {
    %c0 = arith.constant 0 : index
    %c1024 = arith.constant 1024 : index
    %0 = ktdp.construct_memory_view %c1024, sizes: [32, 64], strides: [64, 1] {coordinate_set = #set, memory_space = #ktdp.spyre_memory_space<HBM>} : memref<32x64xf16>
    %1 = ktdp.construct_distributed_memory_view(%0, %0 : memref<32x64xf16>, memref<32x64xf16>) : memref<64x64xf16>
    %2 = ktdp.construct_access_tile %0[%c0, %c0] {access_tile_order = #map, access_tile_set = #set} : memref<32x64xf16> -> !ktdp.access_tile<32x64xindex>
    %3 = ktdp.load %2 : <32x64xindex> -> tensor<32x64xf16>
    ktdp.store %3, %2 : tensor<32x64xf16>, <32x64xindex>
    return
  }
}

@bmahjour
bmahjour force-pushed the bmahjour/fix-build-and-test-issues branch from 7609d76 to ab3fd0d Compare April 21, 2026 15:51
@bmahjour bmahjour changed the title Fix build and test issues Fix LIT test issue Apr 21, 2026
@bmahjour

Copy link
Copy Markdown
Collaborator Author

@bmahjour just to follow up on your previous comment

I tried building with LLVM v22.1 4434dabb69916856b824f68a64b029c67175e532 but I'm now getting a different error:

I triggered a build on the above commit

* the LLVM build is [here](https://github.com/fabianlim/ktir-mlir-frontend/actions/runs/24588998228/job/71905475749)

* the KTIR build downloads the `4434dabb` build [here](https://github.com/fabianlim/ktir-mlir-frontend/actions/runs/24588998228/job/71916321883#step:8:13)

* the tests all [pass](https://github.com/fabianlim/ktir-mlir-frontend/actions/runs/24588998228/job/71916321883#step:12:1)

I will try running your command next ./bin/ktir-opt ../test/Ktdp/basic.mlir and checking if I get your error

Update: hmm yes it seems to work for me
build/bin/ktir-opt test/Ktdp/basic.mlir

Thanks for trying @fabianlim. I think the issue may be hidden depending on linker behavior. Please see #9 for follow up on this issue.

@bmahjour

bmahjour commented Apr 21, 2026

Copy link
Copy Markdown
Collaborator Author

Regarding investigation from Claude, it seems missguided. We are seeing the issue going from LLVM 20 to 22, but it's talking about a difference between v19 to 20. The explanation about order of attributes seems irrelevant as well, since we observe difference in order of operands of an affine add expression, while they are talking about difference in order of two separate affine attributes.

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.

2 participants