[CIR][CodeGen][Lowering] Supports arrays with trailing zeros #393
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ With the latest revision this PR passed the C/C code formatter. |
gitoleg
force-pushed
the
trailing-zero
branch
from
2f98faf
to
11b8b05
Compare
bcardosolopes
requested changes
Jan 16, 2024
clang/test/CIR/CodeGen/const-array.c
Outdated
|
|
||
| // CHECK: cir.func {{.*@foo}} | ||
| // CHECK: %0 = cir.alloca !cir.array<!s32i x 10>, cir.ptr <!cir.array<!s32i x 10>>, ["a"] {alignment = 16 : i64} | ||
| // CHECK: %1 = cir.const(#cir.const_array<[#cir.int<1> : !s32i], 9 zeros> : !cir.array<!s32i x 10>) : !cir.array<!s32i x 10> |
There was a problem hiding this comment.
Thanks for working on this. I like how this simplifies and allows postponing all different initialization heuristics for structs (as you linked for unimplemented features).
Is there any special reason to print "9 zeros" instead of just "trailingzeros"? Are there other uses for the actual value?
I can see this being just implemented as a UnitAttr, which could optionally print "trailingzeros". Sounds like mlir::cir::ArrayType and elts can be used to compute how many trailing zeros there are when lowering?
There was a problem hiding this comment.
@bcardosolopes
yes, it's better! Though I used just bool instead of UnitAttr, hope it's ok)
So let's take a look again!
lanza
force-pushed
the
main
branch
2 times, most recently
from
4e069c6
to
79d4dc7
Compare
|
@lanza just rebased and this has the side effect of breaking GH PR workflow, making it impossible to review, apologies. Please rebase! |
gitoleg
force-pushed
the
trailing-zero
branch
from
d423157
to
18f2b76
Compare
@bcardosolopes done! |
bcardosolopes
requested changes
Jan 31, 2024
| } | ||
|
|
||
| void ConstArrayAttr::print(::mlir::AsmPrinter &printer) const { | ||
| printer << "<"; | ||
| printer.printStrippedAttrOrType(getElts()); | ||
| if (auto zeros = getTrailingZerosNum()) | ||
| printer << ", trailingZeros"; |
There was a problem hiding this comment.
For printed CIR I suggest we use trailing_zeros or trailingzeros.
| if (auto str = elts.dyn_cast<mlir::StringAttr>()) | ||
| return typeSize - str.size(); | ||
| else | ||
| return typeSize - getElts().cast<mlir::ArrayAttr>().size(); |
There was a problem hiding this comment.
Since the trailing number of zeros is a constant, can we compute this during attr creation and only return a value here instead?
|
@bcardosolopes done! |
lanza
pushed a commit
that referenced
this pull request
Mar 23, 2024
This PR adds support for constant arrays with trailing zeros. The original `CodeGen` does the following: once a constant array contain trailing zeros, a struct with two members is generated: initialized elements and `zeroinitializer` for the remaining part. And depending on some conditions, `memset` or `memcpy` are emitted. In the latter case a global const array is created. Well, we may go this way, but it requires us to implement [features](https://github.com/llvm/clangir/blob/main/clang/lib/CIR/CodeGen/CIRGenDecl.cpp#L182) that are not implemented yet. Another option is to add one more parameter to the `constArrayAttr` and utilize it during the lowering. So far I chose this way, but if you have any doubts, we can discuss here. So we just emit constant array as usually and once there are trailing zeros, lower this arrray (i.e. an attribute) as a value. I added a couple of tests and will add more, once we agree on the approach. So far I marked the PR as a draft one.
eZWALT
pushed a commit
to eZWALT/clangir
that referenced
this pull request
Mar 24, 2024
This PR adds support for constant arrays with trailing zeros. The original `CodeGen` does the following: once a constant array contain trailing zeros, a struct with two members is generated: initialized elements and `zeroinitializer` for the remaining part. And depending on some conditions, `memset` or `memcpy` are emitted. In the latter case a global const array is created. Well, we may go this way, but it requires us to implement [features](https://github.com/llvm/clangir/blob/main/clang/lib/CIR/CodeGen/CIRGenDecl.cpp#L182) that are not implemented yet. Another option is to add one more parameter to the `constArrayAttr` and utilize it during the lowering. So far I chose this way, but if you have any doubts, we can discuss here. So we just emit constant array as usually and once there are trailing zeros, lower this arrray (i.e. an attribute) as a value. I added a couple of tests and will add more, once we agree on the approach. So far I marked the PR as a draft one.
lanza
pushed a commit
that referenced
this pull request
Apr 29, 2024
This PR adds support for constant arrays with trailing zeros. The original `CodeGen` does the following: once a constant array contain trailing zeros, a struct with two members is generated: initialized elements and `zeroinitializer` for the remaining part. And depending on some conditions, `memset` or `memcpy` are emitted. In the latter case a global const array is created. Well, we may go this way, but it requires us to implement [features](https://github.com/llvm/clangir/blob/main/clang/lib/CIR/CodeGen/CIRGenDecl.cpp#L182) that are not implemented yet. Another option is to add one more parameter to the `constArrayAttr` and utilize it during the lowering. So far I chose this way, but if you have any doubts, we can discuss here. So we just emit constant array as usually and once there are trailing zeros, lower this arrray (i.e. an attribute) as a value. I added a couple of tests and will add more, once we agree on the approach. So far I marked the PR as a draft one.
lanza
pushed a commit
that referenced
this pull request
Apr 29, 2024
This PR adds support for constant arrays with trailing zeros. The original `CodeGen` does the following: once a constant array contain trailing zeros, a struct with two members is generated: initialized elements and `zeroinitializer` for the remaining part. And depending on some conditions, `memset` or `memcpy` are emitted. In the latter case a global const array is created. Well, we may go this way, but it requires us to implement [features](https://github.com/llvm/clangir/blob/main/clang/lib/CIR/CodeGen/CIRGenDecl.cpp#L182) that are not implemented yet. Another option is to add one more parameter to the `constArrayAttr` and utilize it during the lowering. So far I chose this way, but if you have any doubts, we can discuss here. So we just emit constant array as usually and once there are trailing zeros, lower this arrray (i.e. an attribute) as a value. I added a couple of tests and will add more, once we agree on the approach. So far I marked the PR as a draft one.
eZWALT
pushed a commit
to eZWALT/clangir
that referenced
this pull request
Apr 29, 2024
This PR adds support for constant arrays with trailing zeros. The original `CodeGen` does the following: once a constant array contain trailing zeros, a struct with two members is generated: initialized elements and `zeroinitializer` for the remaining part. And depending on some conditions, `memset` or `memcpy` are emitted. In the latter case a global const array is created. Well, we may go this way, but it requires us to implement [features](https://github.com/llvm/clangir/blob/main/clang/lib/CIR/CodeGen/CIRGenDecl.cpp#L182) that are not implemented yet. Another option is to add one more parameter to the `constArrayAttr` and utilize it during the lowering. So far I chose this way, but if you have any doubts, we can discuss here. So we just emit constant array as usually and once there are trailing zeros, lower this arrray (i.e. an attribute) as a value. I added a couple of tests and will add more, once we agree on the approach. So far I marked the PR as a draft one.
lanza
pushed a commit
that referenced
this pull request
Apr 29, 2024
This PR adds support for constant arrays with trailing zeros. The original `CodeGen` does the following: once a constant array contain trailing zeros, a struct with two members is generated: initialized elements and `zeroinitializer` for the remaining part. And depending on some conditions, `memset` or `memcpy` are emitted. In the latter case a global const array is created. Well, we may go this way, but it requires us to implement [features](https://github.com/llvm/clangir/blob/main/clang/lib/CIR/CodeGen/CIRGenDecl.cpp#L182) that are not implemented yet. Another option is to add one more parameter to the `constArrayAttr` and utilize it during the lowering. So far I chose this way, but if you have any doubts, we can discuss here. So we just emit constant array as usually and once there are trailing zeros, lower this arrray (i.e. an attribute) as a value. I added a couple of tests and will add more, once we agree on the approach. So far I marked the PR as a draft one.
bruteforceboy
pushed a commit
to bruteforceboy/clangir
that referenced
this pull request
Oct 2, 2024
This PR adds support for constant arrays with trailing zeros. The original `CodeGen` does the following: once a constant array contain trailing zeros, a struct with two members is generated: initialized elements and `zeroinitializer` for the remaining part. And depending on some conditions, `memset` or `memcpy` are emitted. In the latter case a global const array is created. Well, we may go this way, but it requires us to implement [features](https://github.com/llvm/clangir/blob/main/clang/lib/CIR/CodeGen/CIRGenDecl.cpp#L182) that are not implemented yet. Another option is to add one more parameter to the `constArrayAttr` and utilize it during the lowering. So far I chose this way, but if you have any doubts, we can discuss here. So we just emit constant array as usually and once there are trailing zeros, lower this arrray (i.e. an attribute) as a value. I added a couple of tests and will add more, once we agree on the approach. So far I marked the PR as a draft one.
Hugobros3
pushed a commit
to shady-gang/clangir
that referenced
this pull request
Oct 2, 2024
This PR adds support for constant arrays with trailing zeros. The original `CodeGen` does the following: once a constant array contain trailing zeros, a struct with two members is generated: initialized elements and `zeroinitializer` for the remaining part. And depending on some conditions, `memset` or `memcpy` are emitted. In the latter case a global const array is created. Well, we may go this way, but it requires us to implement [features](https://github.com/llvm/clangir/blob/main/clang/lib/CIR/CodeGen/CIRGenDecl.cpp#L182) that are not implemented yet. Another option is to add one more parameter to the `constArrayAttr` and utilize it during the lowering. So far I chose this way, but if you have any doubts, we can discuss here. So we just emit constant array as usually and once there are trailing zeros, lower this arrray (i.e. an attribute) as a value. I added a couple of tests and will add more, once we agree on the approach. So far I marked the PR as a draft one.
keryell
pushed a commit
to keryell/clangir
that referenced
this pull request
Oct 19, 2024
This PR adds support for constant arrays with trailing zeros. The original `CodeGen` does the following: once a constant array contain trailing zeros, a struct with two members is generated: initialized elements and `zeroinitializer` for the remaining part. And depending on some conditions, `memset` or `memcpy` are emitted. In the latter case a global const array is created. Well, we may go this way, but it requires us to implement [features](https://github.com/llvm/clangir/blob/main/clang/lib/CIR/CodeGen/CIRGenDecl.cpp#L182) that are not implemented yet. Another option is to add one more parameter to the `constArrayAttr` and utilize it during the lowering. So far I chose this way, but if you have any doubts, we can discuss here. So we just emit constant array as usually and once there are trailing zeros, lower this arrray (i.e. an attribute) as a value. I added a couple of tests and will add more, once we agree on the approach. So far I marked the PR as a draft one.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds support for constant arrays with trailing zeros.
The original
CodeGendoes the following: once a constant array contain trailing zeros, a struct with two members is generated: initialized elements andzeroinitializerfor the remaining part. And depending on some conditions,memsetormemcpyare emitted. In the latter case a global const array is created.Well, we may go this way, but it requires us to implement features that are not implemented yet.
Another option is to add one more parameter to the
constArrayAttrand utilize it during the lowering. So far I chose this way, but if you have any doubts, we can discuss here. So we just emit constant array as usually and once there are trailing zeros, lower this arrray (i.e. an attribute) as a value.I added a couple of tests and will add more, once we agree on the approach. So far I marked the PR as a draft one.