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] Move p4tools.def to common/ir.def, introduce a hashing function for state and symbolic variables. #4667

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

fruffy
Copy link
Collaborator

@fruffy fruffy commented May 17, 2024

Remove one more piece of boost in the compiler.

This time we replace some boost containers with Abseil flat maps and sets. For that to work we needed to introduce hashers and comparison function for some of the special P4Tools variables. We also need custom comparators and hashers because we only want to compare the labels and names of the variables, not the types. The hashers use hash_combine from our hash utilities.

Also fix a small code in the Python PTF scripts that led to spurious failures (trying to remove a log file that was already removed).

@fruffy fruffy added the p4tools Topics related to the P4Tools back end label May 17, 2024
@fruffy fruffy force-pushed the fruffy/testgen_boost_pruning branch 2 times, most recently from 79f11bd to 9cee939 Compare May 17, 2024 20:53
@fruffy fruffy marked this pull request as ready for review May 20, 2024 14:40
ir/solver.h Outdated Show resolved Hide resolved
Comment on lines 129 to 52
uint64_t hash() const {
return hash(0, ref);
}

uint64_t hash(uint64_t seed, const IR::Expression *expression) const {
// expression is a Member.
if (const auto *member = expression->to<IR::Member>()) {
return hash(seed, member);
}
// expression is a PathExpression.
if (const auto *pathExpression = expression->to<IR::PathExpression>()) {
return hash(seed, pathExpression);
}
// expression is a ArrayIndex.
if (const auto *arrayIndex = expression->to<IR::ArrayIndex>()) {
return hash(seed, arrayIndex);
}
BUG("Either %1% of type %2% is not a valid StateVariable", expression,
expression->node_type_name());
}

uint64_t hash(uint64_t seed, const IR::Member *member) const {
return hash(Util::hash_combine(seed, std::hash<cstring>()(member->member)), member->expr);
}

uint64_t hash(uint64_t seed, const IR::PathExpression *pathExpression) const {
return Util::hash_combine(seed, std::hash<cstring>()(pathExpression->path->name));
}

uint64_t hash(uint64_t seed, const IR::ArrayIndex *arrayIndex) const {
const auto *arrayIndexVal = arrayIndex->right->to<IR::Constant>();
BUG_CHECK(
arrayIndexVal != nullptr,
"Value %1% is not a constant. Only constants are supported as part of a state variable.",
arrayIndex->right);
return hash(Util::hash_combine(seed, std::hash<big_int>()(arrayIndexVal->value)),
arrayIndex->left);
}

Copy link
Contributor

Choose a reason for hiding this comment

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

I wonder if it would be better to implement the hash outside the IR classes. Having the multiple public overloads there on the public interface seems messy to me.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My problem with that approach is that it can be tedious to find the implementation. But in this case it makes sense, I just need to implement a new class.

Copy link
Contributor

Choose a reason for hiding this comment

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

But in this case it makes sense, I just need to implement a new class.

Any updates on this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This PR is still blocked until #4768 is resolved.

@fruffy fruffy force-pushed the fruffy/testgen_boost_pruning branch from 9cee939 to 252c182 Compare August 1, 2024 12:14
@fruffy fruffy marked this pull request as draft August 1, 2024 12:57
@fruffy fruffy force-pushed the fruffy/testgen_boost_pruning branch from 252c182 to 3f487da Compare August 3, 2024 09:55
@fruffy fruffy changed the title Replace P4Tools boost maps with Abseil maps. [P4Testgen] Move p4tools.def to common/ir.def, introduce a hashing function for state and symbolic variables. Aug 3, 2024
@fruffy fruffy force-pushed the fruffy/testgen_boost_pruning branch 3 times, most recently from f2691ff to 08e01d0 Compare August 3, 2024 11:17
@fruffy fruffy force-pushed the fruffy/testgen_boost_pruning branch from 08e01d0 to ff5b0e9 Compare August 15, 2024 08:09
@fruffy fruffy force-pushed the fruffy/testgen_boost_pruning branch from ff5b0e9 to 667d482 Compare August 15, 2024 09:27
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.

3 participants