-
Notifications
You must be signed in to change notification settings - Fork 445
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
Introduce a frontend policy as a customization point for frontend #4406
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good (probably better than what is in #4402, and would make that unnecessary).
Part of the issue is that the frontend ends up doing too much -- it really should not be doing inlining at all and probably should not be doing constant folding, except to the extent that it is needed for type checking and inferencing. That's really the sticking point, as we need to fold expressions used in type contexts (eg, in the N
in bit<N>
and int<N>
), which may also require some inlining to squeeze everything down to a constant. I'm not sure what the right answer is.
8948294
to
d94fb56
Compare
Thank you.
I agree. A lot can be moved to midend, but we need to be able to type-check in frontend. But apparently we do run type checking before constant folding so maybe it is not needed as much? Mauybe this is somewhat related to @jnfoster's p4lang/p4-spec#1213, but haven't read through that proposal. The p4c frontend does a lot of things that would be considered "midend" by many other compilers. And it seems like you are hitting cases where the frontend is not universal and applicable to all the p4-based tools (it would be quite interesting to know a bit more on what they are, if that can be said publicly). Thinking about it, p4Tools/p4Testgen could probably also benefit from having non-inlined code -- there would be some cost of course, but at least it would produce more human-readable traces. I think the first step towards a solution would be to describe what the current frontend does. Just at a glance, we might want to split it into a frontend (that basically just checks the P4 is valid and inserts things like implicit cast) and a "flattener" that produces a flat P4 code without function, parser and control calls (with table / action and extern calls only). But the main probably could be that if someone skips the flattening, then a lot of the midend passes would not work properly. |
d94fb56
to
0b8349a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
167f407
to
1e35b94
Compare
... and add a new one for optimizations
38c6f18
to
0902ee2
Compare
One thing we could is to at least start to remove passes from the front end. I think a lot of them have been added at random and may have little effect. Thankfully, we do have the reference files to make sure transformations are semantics-preserving.
Yes, there will be quite a bit of work for that. We rely a lot on the control-flow optimizations of |
Pass the ConstantFoldingPolicy instance to the ConstantFolding instance in SpecializeAll. This was missed in #4406.
Pass the ConstantFoldingPolicy instance to the ConstantFolding instance in SpecializeAll. This was missed in #4406.
Pass the ConstantFoldingPolicy instance to the ConstantFolding instance in SpecializeAll. This was missed in #4406.
Pass the ConstantFoldingPolicy instance to the ConstantFolding instance in SpecializeAll. This was missed in #4406.
) * Add constant folding policy with hook for the use case in #4402 * Add frotend policy to make unify access to frotend customizations ... and add a new one for optimizations * Fix instantiation of frontend in TC * Fix instatiation of frotend in tests * MEV backport: - added optimizationLevel to CompilerOptions (port from future)
Pass the ConstantFoldingPolicy instance to the ConstantFolding instance in SpecializeAll. This was missed in #4406.
Breaking change (dev/test): Side-effects ordering is enabled in frontend tests. This was done by mistake here, but it is probably a good thing, see #4433 for more details.
This was spurred by #4402, but the scope is a bit more general. I've tried to allow customization (almost) all the customizations of frontend into one class that a backend tool can override to tweak certain behaviours.
On top of the things that were already there, and the customization of constant folding that @grg had in #4402, I've added a hook for deciding if optimizations should run -- that is a use case that I have.