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

[P4Testgen] Add a compiler pass to resolve Type_Name in StructExpressions. #4215

Merged
merged 3 commits into from
Nov 21, 2023

Conversation

fruffy
Copy link
Collaborator

@fruffy fruffy commented Oct 31, 2023

In some cases, one may want to create a struct expression, which contains the relevant type directly, without having to use a Type_Name as indirection. This PR relaxes the constraints around the creation of struct expressions to make this possible. We simply add a new parameter that allows us to toggle whether or not to accept IR::Type_StructLike types.

When resolving struct expressions, simply preserve Type_Name for structType but assign the resolved type to type. This works for all dependent PRs and preserves functionality of passes such as toP4 etc.

@fruffy fruffy added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Nov 3, 2023
@fruffy
Copy link
Collaborator Author

fruffy commented Nov 15, 2023

@mihaibudiu I updated the constructor to include an optional structOkay boolean.

@mihaibudiu
Copy link
Contributor

this can't be the whole PR, there is no use of this flag.

@fruffy
Copy link
Collaborator Author

fruffy commented Nov 20, 2023

The flag is used in #4231 and #4255. And an extension which is not part of the main repository here: https://github.com/fruffy/flay

I factored out the change to make the PR more readable.

I could add a compiler pass which resolves Type_Name for StructExpressions. It would make use of this flag. It might be useful for this PR, too: #4225 (comment)

@mihaibudiu
Copy link
Contributor

The pass which resolves the name is called ResolveReferences, it's perhaps the oldest pass in the compiler.
If a struct has no name there is nothing to resolve.

@@ -470,7 +470,7 @@ class StructExpression : Expression {
validate {
components.check_null(); components.validate();
BUG_CHECK(structType == nullptr || structType->is<IR::Type_Name>() ||
structType->is<IR::Type_Specialized>(),
structType->is<IR::Type_Specialized>() || structType->is<IR::Type_StructLike>(),
Copy link
Contributor

Choose a reason for hiding this comment

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

why do you want this?
There is a difference between a struct declaration (Type_Struct) and a reference to a type by name (Type_Name).
It is more annoying to have to resolve the type everytime, but it is the right distinction.
Without this many passes would break.
Perhaps the type field holds the information you want.

@fruffy
Copy link
Collaborator Author

fruffy commented Nov 20, 2023

The pass which resolves the name is called ResolveReferences, it's perhaps the oldest pass in the compiler.
If a struct has no name there is nothing to resolve.

Thanks!

The idea is to resolve all Type_Name past the mid end since they serve no purpose after the passes have completed. This way we can access any type of any expression directly without requiring a map access. Any IR::Expression will have a fully usable type.

@mihaibudiu
Copy link
Contributor

Various backends execute additional transformation passes after the mid-end.
Moreover, this may break to toP4 pass.

@fruffy fruffy changed the title Relax constraints for valid types of struct expressions. [P4Testgen] Add a compiler pass to resolve Type_Name in StructExpressions. Nov 21, 2023
@fruffy
Copy link
Collaborator Author

fruffy commented Nov 21, 2023

@mihaibudiu I believe I found a more palatable solution which does not affect the ir or the rest of the compiler. When resolving struct expressions, I simply preserve Type_Name for structType but assign the resolved type to type. This works for all dependent PRs and preserves functionality of passes such as toP4 etc.

It essentially exploits the inconsistency of type and structType in StructExpressions.

@fruffy fruffy added p4tools Topics related to the P4Tools back end and removed core Topics concerning the core segments of the compiler (frontend, midend, parser) labels Nov 21, 2023
@fruffy fruffy merged commit 85c22c1 into main Nov 21, 2023
13 checks passed
@fruffy fruffy deleted the fruffy/relax_struct_type branch November 21, 2023 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p4tools Topics related to the P4Tools back end
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants