Skip to content
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

ENH: Update MLIR backend to LLVM 20.dev #787

Closed
wants to merge 2 commits into from
Closed

Conversation

mtsokol
Copy link
Collaborator

@mtsokol mtsokol commented Oct 3, 2024

Hi @hameerabbasi,

This PR updates MLIR backend to current LLVM 20.dev (so main branch) experimentally:

  • I ran it locally against latest LLVM version.
  • Moved COO format to SoA convention link.
  • Added format attribute to Tensor class.
  • Updated tensor.empty call link.

As you can see it fixes a bunch of skips in the test suite: sparse/mlir_backend/tests/test_simple.py

  • Dense+Dense and COO+COO now works.
  • Reshaping Dense and COO formats also works.

It's thanks to changes already present in main branch, added after 19.x branched, and:

Maybe one thing that we can consider is to have a nightly label for MLIR Python bindings feedstock some time in the future? Otherwise, I guess 20.x will branch in ~6months?

@mtsokol mtsokol added the enhancement Indicates new feature requests label Oct 3, 2024
@mtsokol mtsokol self-assigned this Oct 3, 2024
@hameerabbasi
Copy link
Collaborator

I think the best option is backporting these PRs as they are blockers and real issues.

@mtsokol
Copy link
Collaborator Author

mtsokol commented Oct 3, 2024

I asked about it here: llvm/llvm-project#109135 (comment).

But reshaping and adding Dense formats will stay broken until first 20 RC as it happens to be fixed in the current main.

@hameerabbasi
Copy link
Collaborator

In that case we will need to build LLVM in CI (which is quite complicated) and it absolutely needs to be cached.

Copy link

codspeed-hq bot commented Oct 3, 2024

CodSpeed Performance Report

Merging #787 will degrade performances by 32.04%

Comparing llvm-nightly-test (32abcf0) with main (db60537)

Summary

❌ 10 regressions
✅ 330 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main llvm-nightly-test Change
test_index_slice[side=100-rank=2-format='gcxs'] 2.2 ms 2.7 ms -19.97%
test_elemwise[f=<built-in function add>-backend='Finch'-side=1000] 1.5 ms 1.8 ms -16.66%
test_elemwise[f=<built-in function add>-backend='Finch'-side=100] 713.7 µs 1,037.5 µs -31.21%
test_elemwise[f=<built-in function add>-backend='Finch'-side=500] 882.4 µs 1,175.5 µs -24.94%
test_elemwise[f=<built-in function gt>-backend='Finch'-side=1000] 1.4 ms 1.7 ms -13.81%
test_elemwise[f=<built-in function gt>-backend='Finch'-side=100] 708.9 µs 940.2 µs -24.61%
test_elemwise[f=<built-in function gt>-backend='Finch'-side=500] 879.6 µs 1,109.9 µs -20.75%
test_elemwise[f=<built-in function mul>-backend='Finch'-side=1000] 736.4 µs 1,068.9 µs -31.11%
test_elemwise[f=<built-in function mul>-backend='Finch'-side=100] 694 µs 1,021.2 µs -32.04%
test_elemwise[f=<built-in function mul>-backend='Finch'-side=500] 713.3 µs 1,045.9 µs -31.8%

@mtsokol
Copy link
Collaborator Author

mtsokol commented Oct 23, 2024

In that case we will need to build LLVM in CI (which is quite complicated) and it absolutely needs to be cached.

Right, in #799 I added caching with ccache and LLVM builds in 10min instead of 2h.

@hameerabbasi
Copy link
Collaborator

Closing in favour of #799.

@mtsokol mtsokol deleted the llvm-nightly-test branch January 2, 2025 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Indicates new feature requests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants