Handle correctly general switch statements when simplifying #3624
Conversation
Signed-off-by: Mihai Budiu <mbudiu@vmware.com>
|
Does this handle side effects in the switch expression that is: And still have the table apply there? |
|
It should, if there is a method call in the switch statement it is preserved. |
|
I don't think this is conservative enough, since it is also run before the side-effect-ordering pass. |
Signed-off-by: Mihai Budiu <mbudiu@vmware.com>
| "%1%: expected a table invocation", expr->expr); | ||
| auto mce = expr->expr->to<IR::MethodCallExpression>(); | ||
| return new IR::MethodCallStatement(mce->srcInfo, mce); | ||
| // If this is a table application remove the switch altogether but keep |
There was a problem hiding this comment.
Do we have a test for this?
There was a problem hiding this comment.
Also another test for a function call with side-effects could be useful.
There was a problem hiding this comment.
issue3619-1.p4 is testing the side-effect.
There was a problem hiding this comment.
But that function call is side-effect free. Also this does not include a table.apply(), right?
There was a problem hiding this comment.
This pass does not know that the function has no side effects. All tests with table applications would qualify for the second case.
There was a problem hiding this comment.
if (SideEffects::check(statement->expression, this, refMap, typeMap))
// This can happen if this pass is run before SideEffectOrdering.
return statement;
I am trying to understand this segment. If the function call has side effects, ideally only the method call statement should remain, right? I do not think we have a test for this.
Similarly,
if (TableApplySolver::isActionRun(statement->expression, refMap, typeMap)) {
should check whether there is an action_run call and then only leave the table apply statement.
I assume we have tests already for this case? Since this was the default behavior.
There was a problem hiding this comment.
If the argument of the switch may have side-effects then we leave the whole switch unchaged. This pass is invoked again later, after SideEffectOrdering has removed calls with side effects from switch statements (all except table.apply().action_run), and that case will never occur again.
Signed-off-by: Mihai Budiu mbudiu@vmware.com
Fixes #3619