search

iree-org / iree

A retargetable MLIR-based machine learning compiler and runtime toolkit.
http://iree.dev/
Apache License 2.0
2.85k stars 620 forks source link

[i1] Do not emit `arith.trunci` cast from i1 to i1 #19176

Closed lialan closed 2 days ago

lialan commented 5 days ago

arith.trunci does not allow cast to same type

lialan commented 5 days ago

@rohan-tan-bhowmik I am trying to fix an issue while making i1 attention works. However I have a problem: I don't know where to find an minimized test case for it as after decomposition my simple test cases are turned into big ones. Original commit does not show a lot of helpful info as well.

I am wondering if you can give me a pointer to where I can get a test case for it?

rohan-tan-bhowmik commented 5 days ago

Hi @lialan! I had created a repository for specifically testing different variants of masked and causal attention: https://github.com/rohan-tan-bhowmik/iree-masked-attention-test. Please check it out and let me know if you have any questions about it. If this doesn't seem to help, let me know as well :)

lialan commented 5 days ago

Hi @lialan! I had created a repository for specifically testing different variants of masked and causal attention: https://github.com/rohan-tan-bhowmik/iree-masked-attention-test. Please check it out and let me know if you have any questions about it. If this doesn't seem to help, let me know as well :)

@rohan-tan-bhowmik Thanks! I am actually already using your repo to test i1 attention support. I already hit a problem: the emitted iree_linalg_ext.attention now needs to take a region which yields values. But the script does not append a region for it.

I am doing manual changes in my private repo to bypass it, but it would be great to amend that in your repo!

lialan commented 5 days ago

@rohan-tan-bhowmik still, the generated test file is too big a unit test for such a small change. I wonder if you can help me find a unit test for it?

rohan-tan-bhowmik commented 5 days ago

@lialan Hmm, would https://github.com/iree-org/iree/issues/17892 help?

lialan commented 4 days ago

@lialan Hmm, would #17892 help?

@rohan-tan-bhowmik It seems currently it emits attention test but the attention test is missing the optional i1 masking component? I can add one new component to it, it is simple.

Also perhaps I can still use a unit test here? But I think this change is small, I am okay submitting this PR without a test case.

lialan commented 4 days ago

I can compile the out-of-tree repo emitted test with some changes along with this PR and another PR upstream. But it is missing correctness checking mechanism, and also the mask.

Need this test to verify the generated result is sound, looking into it.

rohan-tan-bhowmik commented 2 days ago

@lialan Good work!