[CIR][CodeGen][BugFix] Fixes structures name collisions #844
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
bcardosolopes
approved these changes
Sep 16, 2024
Hugobros3
pushed a commit
to shady-gang/clangir
that referenced
this pull request
Oct 2, 2024
CIR Codegen fails to generate functions with local types with the same
names. For instance, the next code :
```
void foo(int a, float b) {
struct A { int x; };
struct A loc = {a};
{
struct A { float y; };
struct A loc = {b};
}
}
```
fails with on the next assertion: `Unable to find record layout
information for type`. The problem is that we don't create record layout
for the structures with equal names and
`CIRGenTypes::convertRecordDeclType` returns the wrong type for the
second struct type in the example above. This PR fixes this problem.
In the original codegen the call to `Ty->setName(name)` resolves name
collisions and assign a proper name for the type. In our case looks like
we need to use the same approach as we did for the anonymous structures,
i.e. to track the used names in the builder.
Also, I fixed the struct type creation. Previously, the type was created
several times - first in the `CIRGenTypes::convertRecordDeclType` and
then in the `CIRGenTypes::computeRecordLayout`. This is why the indexes
used by the anonymous structures naming had relatively big values and
this is where the most changes on the tests come from.
smeenai
pushed a commit
to smeenai/clangir
that referenced
this pull request
Oct 9, 2024
CIR Codegen fails to generate functions with local types with the same
names. For instance, the next code :
```
void foo(int a, float b) {
struct A { int x; };
struct A loc = {a};
{
struct A { float y; };
struct A loc = {b};
}
}
```
fails with on the next assertion: `Unable to find record layout
information for type`. The problem is that we don't create record layout
for the structures with equal names and
`CIRGenTypes::convertRecordDeclType` returns the wrong type for the
second struct type in the example above. This PR fixes this problem.
In the original codegen the call to `Ty->setName(name)` resolves name
collisions and assign a proper name for the type. In our case looks like
we need to use the same approach as we did for the anonymous structures,
i.e. to track the used names in the builder.
Also, I fixed the struct type creation. Previously, the type was created
several times - first in the `CIRGenTypes::convertRecordDeclType` and
then in the `CIRGenTypes::computeRecordLayout`. This is why the indexes
used by the anonymous structures naming had relatively big values and
this is where the most changes on the tests come from.
smeenai
pushed a commit
to smeenai/clangir
that referenced
this pull request
Oct 9, 2024
CIR Codegen fails to generate functions with local types with the same
names. For instance, the next code :
```
void foo(int a, float b) {
struct A { int x; };
struct A loc = {a};
{
struct A { float y; };
struct A loc = {b};
}
}
```
fails with on the next assertion: `Unable to find record layout
information for type`. The problem is that we don't create record layout
for the structures with equal names and
`CIRGenTypes::convertRecordDeclType` returns the wrong type for the
second struct type in the example above. This PR fixes this problem.
In the original codegen the call to `Ty->setName(name)` resolves name
collisions and assign a proper name for the type. In our case looks like
we need to use the same approach as we did for the anonymous structures,
i.e. to track the used names in the builder.
Also, I fixed the struct type creation. Previously, the type was created
several times - first in the `CIRGenTypes::convertRecordDeclType` and
then in the `CIRGenTypes::computeRecordLayout`. This is why the indexes
used by the anonymous structures naming had relatively big values and
this is where the most changes on the tests come from.
keryell
pushed a commit
to keryell/clangir
that referenced
this pull request
Oct 19, 2024
CIR Codegen fails to generate functions with local types with the same
names. For instance, the next code :
```
void foo(int a, float b) {
struct A { int x; };
struct A loc = {a};
{
struct A { float y; };
struct A loc = {b};
}
}
```
fails with on the next assertion: `Unable to find record layout
information for type`. The problem is that we don't create record layout
for the structures with equal names and
`CIRGenTypes::convertRecordDeclType` returns the wrong type for the
second struct type in the example above. This PR fixes this problem.
In the original codegen the call to `Ty->setName(name)` resolves name
collisions and assign a proper name for the type. In our case looks like
we need to use the same approach as we did for the anonymous structures,
i.e. to track the used names in the builder.
Also, I fixed the struct type creation. Previously, the type was created
several times - first in the `CIRGenTypes::convertRecordDeclType` and
then in the `CIRGenTypes::computeRecordLayout`. This is why the indexes
used by the anonymous structures naming had relatively big values and
this is where the most changes on the tests come from.
lanza
pushed a commit
that referenced
this pull request
Nov 5, 2024
CIR Codegen fails to generate functions with local types with the same
names. For instance, the next code :
```
void foo(int a, float b) {
struct A { int x; };
struct A loc = {a};
{
struct A { float y; };
struct A loc = {b};
}
}
```
fails with on the next assertion: `Unable to find record layout
information for type`. The problem is that we don't create record layout
for the structures with equal names and
`CIRGenTypes::convertRecordDeclType` returns the wrong type for the
second struct type in the example above. This PR fixes this problem.
In the original codegen the call to `Ty->setName(name)` resolves name
collisions and assign a proper name for the type. In our case looks like
we need to use the same approach as we did for the anonymous structures,
i.e. to track the used names in the builder.
Also, I fixed the struct type creation. Previously, the type was created
several times - first in the `CIRGenTypes::convertRecordDeclType` and
then in the `CIRGenTypes::computeRecordLayout`. This is why the indexes
used by the anonymous structures naming had relatively big values and
this is where the most changes on the tests come from.
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.
CIR Codegen fails to generate functions with local types with the same names. For instance, the next code :
fails with on the next assertion:
Unable to find record layout information for type. The problem is that we don't create record layout for the structures with equal names andCIRGenTypes::convertRecordDeclTypereturns the wrong type for the second struct type in the example above. This PR fixes this problem.In the original codegen the call to
Ty->setName(name)resolves name collisions and assign a proper name for the type. In our case looks like we need to use the same approach as we did for the anonymous structures, i.e. to track the used names in the builder.Also, I fixed the struct type creation. Previously, the type was created several times - first in the
CIRGenTypes::convertRecordDeclTypeand then in theCIRGenTypes::computeRecordLayout. This is why the indexes used by the anonymous structures naming had relatively big values and this is where the most changes on the tests come from.