Skip to content

Conversation

ChuanqiXu9
Copy link
Member

Close #957

the previous algorithm to convert a multiple dimension array to a tensor is: fill the value one by one and fill the zero values in conditions. And it has some problems handling the multiple dimension array as above issue shows so that the generated values are not in the same shape with the original array.

the new algorithm here is, full fill the values ahead of time with the correct element size and full fill the values to different slots and we only need to maintain the index to write.

I feel the new version has better performance (avoid allocation) and better readability slightly.

@bcardosolopes bcardosolopes changed the title [clangir] [Lowering] Handling Lowering of multiple dimension array correctly [CIR][Lowering] Handling Lowering of multiple dimension array correctly Oct 11, 2024
@bcardosolopes
Copy link
Member

Code looks good, just need to go over some test failures!

@smeenai
Copy link
Collaborator

smeenai commented Oct 12, 2024

The test failures should have all been fixed by #963, this just needs a rebase.

@ChuanqiXu9 ChuanqiXu9 force-pushed the handling_multiple_dimension_array branch from e213f76 to bcd7986 Compare October 14, 2024 02:29
@ChuanqiXu9
Copy link
Member Author

rebased

Copy link
Collaborator

@smeenai smeenai left a comment

Choose a reason for hiding this comment

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

@bcardosolopes had approved the code changes, so we should be good to merge this.

@smeenai smeenai merged commit eae6288 into llvm:main Oct 14, 2024
6 checks passed
smeenai pushed a commit that referenced this pull request Oct 14, 2024
This is the following up fix for the previous fix
#961

See the attached new test for the reproducer.

Sorry for the initial overlook.
keryell pushed a commit to keryell/clangir that referenced this pull request Oct 19, 2024
…ly (llvm#961)

Close llvm#957

the previous algorithm to convert a multiple dimension array to a tensor
is: fill the value one by one and fill the zero values in conditions.
And it has some problems handling the multiple dimension array as above
issue shows so that the generated values are not in the same shape with
the original array.

the new algorithm here is, full fill the values ahead of time with the
correct element size and full fill the values to different slots and we
only need to maintain the index to write.

I feel the new version has better performance (avoid allocation) and
better readability slightly.
keryell pushed a commit to keryell/clangir that referenced this pull request Oct 19, 2024
This is the following up fix for the previous fix
llvm#961

See the attached new test for the reproducer.

Sorry for the initial overlook.
lanza pushed a commit that referenced this pull request Nov 5, 2024
…ly (#961)

Close #957

the previous algorithm to convert a multiple dimension array to a tensor
is: fill the value one by one and fill the zero values in conditions.
And it has some problems handling the multiple dimension array as above
issue shows so that the generated values are not in the same shape with
the original array.

the new algorithm here is, full fill the values ahead of time with the
correct element size and full fill the values to different slots and we
only need to maintain the index to write.

I feel the new version has better performance (avoid allocation) and
better readability slightly.
lanza pushed a commit that referenced this pull request Nov 5, 2024
This is the following up fix for the previous fix
#961

See the attached new test for the reproducer.

Sorry for the initial overlook.
lanza pushed a commit that referenced this pull request Mar 18, 2025
…ly (#961)

Close #957

the previous algorithm to convert a multiple dimension array to a tensor
is: fill the value one by one and fill the zero values in conditions.
And it has some problems handling the multiple dimension array as above
issue shows so that the generated values are not in the same shape with
the original array.

the new algorithm here is, full fill the values ahead of time with the
correct element size and full fill the values to different slots and we
only need to maintain the index to write.

I feel the new version has better performance (avoid allocation) and
better readability slightly.
lanza pushed a commit that referenced this pull request Mar 18, 2025
This is the following up fix for the previous fix
#961

See the attached new test for the reproducer.

Sorry for the initial overlook.
lanza pushed a commit to lanza/llvm-project that referenced this pull request Aug 11, 2025
This is the following up fix for the previous fix
llvm/clangir#961

See the attached new test for the reproducer.

Sorry for the initial overlook.
terapines-osc-cir pushed a commit to Terapines/clangir that referenced this pull request Sep 2, 2025
…ly (llvm#961)

Close llvm#957

the previous algorithm to convert a multiple dimension array to a tensor
is: fill the value one by one and fill the zero values in conditions.
And it has some problems handling the multiple dimension array as above
issue shows so that the generated values are not in the same shape with
the original array.

the new algorithm here is, full fill the values ahead of time with the
correct element size and full fill the values to different slots and we
only need to maintain the index to write.

I feel the new version has better performance (avoid allocation) and
better readability slightly.
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.

Crash when lowering multiple dimention array
3 participants