-
Notifications
You must be signed in to change notification settings - Fork 448
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
constant folding streamlining, bug fixes, and Doxygen comments (#541)
The previous version of constant folding had two bugs:
It did not fold constants in all cases, or check that declaration constants are initialized with genuine constants. For example, on the input program,
```
const T t = { 32s10, 32s20 };
const S s = { { 32s15, 32s25}, t };
const int<32> x = t.t1;
const int<32> y = s.s1.t2;
const int<32> w = .t.t1;
const T t1 = (T)s.s1;
```
constant folding (with type information) yields:
```
const T t = { 32s10, 32s20 };
const S s = { { 32s15, 32s25 }, t };
const int<32> x = 32s10;
const int<32> y = 32s25;
const int<32> w = 32s10;
const T t1 = { 32s15, 32s25 };
```
Note that the RHS of the declaration of s is not a constant -- it references a variable t.
The root cause of this behavior is due to the use of auxiliary data structure constants: the pass does not ensure that all expressions that are inserted into this map are genuine constants, yet it assumes that getConstants, which returns arbitrary entries from constants, are always a constant. Hence, the check for Declaration_Constants is not correct.
The fix implemented here is to streamline the use of constants so it is used in exactly two places:
1. It is populated with genuine constants in the IR::Declaration_Constant case
2. It is read in the IR::PathExpression (aka variable) case
In addition, when the pass transforms the IR, it attempts to patch the typeMap with type information about the newly inserted nodes. However, the types it inserts are not always correct -- e.g., in particular, in the IR::Member case, when header stack sizes are folded, the type map is updated with `origtype`, the type of the expression, but the arbitrary-precision integer may require a cast. Hence, subsequent type-checking passes will rely on the information in the typeMap and assume that no cast is required. However, if the typeMap is ever cleared, then the typeChecker will mutate the node, triggering an error.
This error was not triggered by any of our current tests -- in examples like p4_16_samples/stack.p4, the IR::Member is dead code and is eliminated before the typeMap is cleared. Hence, the error never manifests. However, a close variant can trigger the error:
```
#include <core.p4>
header h { }
control c(out bit<32> x) {
apply {
h[4] stack;
bit<32> sz = stack.size;
x = sz;
}
}
control Simpler(out bit<32> x);
package top(Simpler ctr);
top(c()) main;
```
The fix implemented here is to change the `typeMap` to `const` and reconstruct it after type checking. In addition, we statically convert header stacks sizes to the appropriate type.
This commit adds a new unit tests that checks for failures involving constant folding and header stacks.
It also adds Doxygen documentation.- Loading branch information
Showing
15 changed files
with
286 additions
and
110 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.