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

Add string literal support in constant folding. #4837

Merged
merged 2 commits into from
Jul 29, 2024
Merged

Conversation

fruffy
Copy link
Collaborator

@fruffy fruffy commented Jul 28, 2024

String literals can also be compared and folded if they are equal or unequal. There might be more cases but I think the changes include the major ones.

Signed-off-by: fruffy <fruffy@nyu.edu>
@fruffy fruffy added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Jul 28, 2024
@fruffy fruffy requested a review from vlstill July 28, 2024 18:29
@fruffy fruffy unassigned asl and vlstill Jul 28, 2024
@fruffy fruffy requested a review from asl July 28, 2024 18:30
@@ -440,6 +441,15 @@ const IR::Node *DoConstantFolding::compare(const IR::Operation_Binary *e) {
}
bool bresult = (left->value == right->value) == eqTest;
return new IR::BoolLiteral(e->srcInfo, IR::Type_Boolean::get(), bresult);
} else if (eleft->is<IR::StringLiteral>()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this be simplified to:

} else if (const auto *left = eleft->to<IR::StringLiteral>()) {
..

This would skip double cast. The code for BoolLiteral likely would need to be fixed as well :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, maybe it would help to add comment to bool eqTest above to clarify that compare is called only for IR::Equ and IR::Neq, so other cases are not possible. It tool me a while to figure out why other IR::Operation_Binary cases are not handled...

Signed-off-by: fruffy <fruffy@nyu.edu>
@fruffy fruffy added this pull request to the merge queue Jul 29, 2024
Merged via the queue into main with commit a722c63 Jul 29, 2024
17 checks passed
@fruffy fruffy deleted the fruffy/constant_folding branch July 29, 2024 18:23
@vlstill
Copy link
Contributor

vlstill commented Aug 12, 2024

Fun thing, P4 spec says there are no operations on string literals... What is the use case for comparing strings btw?

@fruffy
Copy link
Collaborator Author

fruffy commented Aug 12, 2024

Fun thing, P4 spec says there are no operations on string literals...

Hmm, in many cases the compiler optimization passes have more flexibility than what the spec permits, under the assumption that user-facing behavior is not affected.

However, in this case, because ConstantFolding is run before type-checking it could be possible that comparing two string literals could lead to a violation. In the sense that a technically invalid program is not rejected (e.g., an action with a bool parameter and an input like "a" == "b"). I assume this parses just fine.

What is the use case for comparing strings btw?

This is quite useful when working with IR expressions (often generated for static/dynamic analysis). For example, simplifying an expression of the from:
"table_action_drop" = "table_action_pass" ? 1 : 0 to 0.

ConstantFolding and StrengthReduction are both useful for simplifying large, complex expressions because they are self-contained and do not require a type or reference map.

Maybe it makes sense to split ConstantFolding into a minimal, spec-complaint version and a rich, self-contained version that performs simplifications as aggressively as possible.

@vlstill
Copy link
Contributor

vlstill commented Aug 12, 2024

Oh, I see, this is for P4Tools etc. not for actually using it in user-written P4 code.

Maybe it makes sense to split ConstantFolding into a minimal, spec-complaint version and a rich, self-contained version that performs simplifications as aggressively as possible.

Maybe... the difference will be really probably (hopefully) just this.

I'd say that ideally, we would want to type-check the program as one of the very first things, before constant folding. That would require ability to constant-evaluate expressions from the type checker (but I'd guess they would/could be type-checked at the point when they are evaluated!). But rewriting the type checker would be a huge thing. That would even allow constant-folding to be deferred to midend, where it belongs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Topics concerning the core segments of the compiler (frontend, midend, parser)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants