Support for invalid union expressions #3843
Conversation
mihaibudiu
force-pushed
the
invalid-union
branch
2 times, most recently
from
a657b66 to
4b0edb3
Compare
|
Anyone can review this? |
mihaibudiu
force-pushed
the
invalid-union
branch
from
747cb7d to
45ae869
Compare
| The fix bellow, commented-out, causes problems elsewhere. | ||
| https://github.com/p4lang/p4c/issues/3842 | ||
| if (ltype->is<IR::Type_HeaderUnion>()) | ||
| return statement; |
There was a problem hiding this comment.
Where does the fix cause problems? Still relevant?
There was a problem hiding this comment.
Yes, that is still relevant. Without this pass other later midend passes fail.
| @@ -21,6 +21,8 @@ limitations under the License. | |||
|
|
|||
| #include <memory> | |||
| #include <ostream> | |||
| // use in combination with "raise" below | |||
There was a problem hiding this comment.
Can we make this a #define instead of commented code? E.g., IR_NODE_USE_SIGNAL or something similar.
There was a problem hiding this comment.
There is not much benefit, the amount of edits needed on a change are about the same.
| ::error("%1%: Cannot eliminate invalid header union", expression); | ||
| return expression; | ||
| } | ||
| cstring name = refMap->newName("ih"); |
There was a problem hiding this comment.
I don't know what you mean by a "dummy" name. We often generate new names for identifiers, this one is for a variable holding an invalid header.
backends/p4test/CMakeLists.txt
Outdated
| @@ -104,6 +104,9 @@ set (P4_XFAIL_TESTS | |||
| # arguments and with incorrect number of arguments | |||
| testdata/p4_16_samples/pna-example-mirror-packet-error2.p4 | |||
| testdata/p4_16_samples/pna-example-mirror-packet-error3.p4 | |||
| # This test fails only when recompiling the produced output, because | |||
| # of the predication pass | |||
| testdata/p4_16_samples/invalid-union.p4 | |||
There was a problem hiding this comment.
Still relevant now that the pass is gone?
There was a problem hiding this comment.
removed this one.
| } | ||
| if (expr->is<IR::InvalidHeader>() && e->member.name == IR::Type_Header::isValid) return e; | ||
| if (expr->is<IR::InvalidHeaderUnion>() && e->member.name == IR::Type_Header::isValid) return e; | ||
| if (expr->is<IR::Invalid>() && e->member.name == IR::Type_Header::isValid) return e; |
There was a problem hiding this comment.
Why remove the brackets? To be more terse?
What is an instance of expr->is<IR::Invalid>()?
There was a problem hiding this comment.
The literal {#} represents an Invalid expression. Whether it's a header or a union will be decided later.
Signed-off-by: Mihai Budiu <mbudiu@vmware.com>
Signed-off-by: Mihai Budiu <mbudiu@vmware.com>
Signed-off-by: Mihai Budiu <mbudiu@vmware.com>
Signed-off-by: Mihai Budiu <mbudiu@vmware.com>
Signed-off-by: Mihai Budiu <mbudiu@vmware.com>
mihaibudiu
force-pushed
the
invalid-union
branch
from
45ae869 to
784831c
Compare
Signed-off-by: Mihai Budiu <mbudiu@vmware.com>
Signed-off-by: Mihai Budiu <mbudiu@vmware.com>
Signed-off-by: Mihai Budiu mbudiu@vmware.com
However, see #3842; that would need to be fixed to make this complete.