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

Comparisons of list expressions, structure-valued expressions, tuples and structs #3057

Closed
MollyDream opened this issue Jan 27, 2022 · 1 comment
Assignees
Labels
bug This behavior is unintended and should be fixed. core Topics concerning the core segments of the compiler (frontend, midend, parser)

Comments

@MollyDream
Copy link

In testing all the comparisons between list expressions, structure-valued expressions, tuples, and structs in the p4c compiler, we found some examples rejected by the compiler (relates to p4lang/p4-spec#960). Some are likely compiler bugs, while some may not be.

Note: the tests are reordered and renumbered for a cleaner view, so it looks different from p4lang/p4-spec#960.

struct S {
    bit<32> a;
    bit<32> b;
}

control compute(inout hdr h) {
    apply {
        bool b1 = { 1, 2 } == { 1, 2 };

        bool b2 = { a = 8w1, b = 8w2 } == { a = 8w1, b = 8w2 };
         // bool b2_ = { a = 1, b = 2 } == { a = 1, b = 2 }; // p4c: Compiler Bug: Could not find type for <Constant>(11032) 1

         // bool b3 = { a = 1, b = 2 } == {1, 2};  // p4c: error: ==: Cannot unify type Tuple(2) with unknown struct
        bool b4 = (S) { a = 1, b = 2 } == {1, 2};

        bool b5 = (S) { a = 1, b = 2 } == { a = 1, b = 2 };
        // bool b5_ = { a = 1, b = 2 } == (S) { a = 1, b = 2 }; // p4c: error: ==: Cannot unify struct S to unknown struct
       
        bool b6 = (S) { a = 1, b = 2 } == (S) { a = 1, b = 2 };

        S s1 = { a = 1, b = 2 };
        S s2 = { a = 1, b = 2 };
        bool b7 = s1 == { a = 1, b = 2 };
        bool b8 = s1 == (S) { a = 1, b = 2 };
        bool b9 = s1 == s2;

        tuple<bit<8>, bit<8>> t1 = {1, 2};
        tuple<bit<8>, bit<8>> t2 = {1, 2};
        bool b10 = t1 == { 1, 2 };
        bool b11 = t1 == t2;
    }
}

The tests b2_ and b5_ are likely bugs.

  • For b2_, it seems to be some other issue with the compiler since b2 works well.
  • For b5_, it seems weird that the compiler cannot infer the type of the LHS. Given that b5 works well, and there is no rule in the spec enforcing the direction of type inference, I think it is more likely a problem with the compiler.

With the test b3 aside, it seems tempting to summarize the allowed comparisons as:

  • A list expression can be compared with a list expression, a structure-valued expression, a tuple, a struct, or a header for
    equality (==) or inequality (!=)
  • A structure-valued expression can be compared with a list expression, a structure-valued expression, a struct, or a header for
    equality (==) or inequality (!=)
  • Two tuples, two structs, and two headers can be compared for equality (==) or inequality (!=)

But the test b3 is indeed the corner case. Comparing b3 and b4, a list expression can be compared with a structure-valued expression only when typRef is explicit. We are still unsure whether b3 should be allowed or not: if yes, then the compiler needs to be fixed; otherwise, we need a more careful specification in the spec to explain this well.

@mihaibudiu mihaibudiu self-assigned this Jan 27, 2022
@mihaibudiu mihaibudiu added the bug This behavior is unintended and should be fixed. label Jan 27, 2022
milossyrmia added a commit to milossyrmia/p4c that referenced this issue Mar 11, 2022
… and structs p4lang#3057

* I corrected the code so that the compiler bug does not occur when comparing structures of unknown type, but to report an error

* I enabled a comparison between the structure and the cast structure so that cast can be put on either side

* Added tests
milossyrmia added a commit to milossyrmia/p4c that referenced this issue Mar 14, 2022
… and structs p4lang#3057

* I corrected the code so that the compiler bug does not occur when comparing structures of unknown type, but to report an error

* I enabled a comparison between the structure/list expression so that the cast can be put on either side. Before this change, if the cast was on the right side of the comparison operator error was reported.

* Added tests which verify the above
milossyrmia added a commit to milossyrmia/p4c that referenced this issue Mar 14, 2022
… and structs p4lang#3057

* I corrected the code so that the compiler bug does not occur when comparing structures of unknown type, but to report an error

* I enabled a comparison between the structure/list expression so that the cast can be put on either side. Before this change, if the cast was on the right side of the comparison operator error was reported.

* Added tests which verify the above
milossyrmia added a commit to milossyrmia/p4c that referenced this issue Mar 15, 2022
… and structs p4lang#3057

* I corrected the code so that the compiler bug does not occur when comparing structures of unknown type, but to report an error

* I enabled a comparison between the structure/list expression so that the cast can be put on either side. Before this change, if the cast was on the right side of the comparison operator error was reported.

* Added tests which verify the above
mihaibudiu pushed a commit that referenced this issue Mar 17, 2022
… and structs #3057 (#3122)

* I corrected the code so that the compiler bug does not occur when comparing structures of unknown type, but to report an error

* I enabled a comparison between the structure/list expression so that the cast can be put on either side. Before this change, if the cast was on the right side of the comparison operator error was reported.

* Added tests which verify the above
rst0git added a commit to rst0git/p4c that referenced this issue Apr 23, 2022
Changelog:

- Extend eBPF kernel target with support for additional BPF helpers and more types of BPF maps (3119)
- loop_revisit method for dealing with recursive loops in the IR (3106)
- Added nullptr checks for l/rtype (3138)
- DPDK: Implementation for non byte aligned metadata and header fields (3114)
- Fix expansion of signed ranges to masks (3212)
- Forbid egress pipeline in dpdk by default (3104)
- Point to PSA/eBPF implementation in main README (3244)
- Remove unnecessary check in conversion of log_msg to a JSON (3067)
- Represent unary plus in the IR (3157)
- Use 0 as action ID for NoAction and refactor eBPF table implementation (3129)
- Add PTF test infra for eBPF backend (3158)
- Add support for ActionSelector extern to PSA/eBPF backend (3216)
- Add support for Checksum extern and CRC16/32 algorithms to eBPF backend (3167)
- Add support for parser value_set to eBPF backend (3235)
- DPDK Backend: Insert table keys generated by compiler closer to the standard and user metadata (3160)
- Fix cpplint errors for dpdk sources (3111)
- P4C-DPDK: Emit error when struct fields >64-bit is present in the P4 (3217)
- Replace pna direction with register read (3224)
- Solving problems with Header Union verify function (reopened) (3214)
- Add --xdp2tc compiler flag for eBPF backend (3187)
- DPDK: Add support for "mirror_packet" PNA extern (3128)
- Eliminate typedefs in bmv2-ss backend (3123)
- Fix off-by-one bug in source file position calculation (3124)
- Add support for Counter extern to PSA/eBPF backend (3165)
- Add support for Register extern to PSA/eBPF backend (3202)
- Comparisons of list expressions, structure-valued expressions, tuples and structs p4lang#3057 (3122)
- Disable 'unused' warning for some psa.p4 externs (3147)
- [dpdk] Shorten the Identifer name, including dots(.) in Member expression (3175)
- Improve error message for count() in dpdk (3101)
- improving the driver re: the checking of input pathnames, improving error messages, trying to prevent misleading error - ssages. (3218)
- Insert type specializations before functions (3207)
- Moved FindRecirculated from .cpp to .h (3088)
- Add Digest extern to PSA/eBPF backend (3164)
- DPDK: Fix core dump while generating bfrt json for PNA programs with action selector (3115)
- Improve efficiency of range translation to mask  (3133)
- PSA-eBPF: Add missing header name substitution in deparser (3162)
- Split psa_switch.h into two files (3144)
- Add debug messages for generated eBPF programs (3097)
- Compile the P4 compiler tools in host configuration (3148)
- Do not allow casts to struct types (3234)
- Fix Context json to emit target_action_name and NoAction (3213)
- Fixed underflow during row-access in the SymBitMatrix (3143) (3169)
- from1.0: fix register type signess (3117)
- Removed execute permission bits from all 3 of the P4 files that had them.
- typeChecker: make error message less confusing (3152)
- Enable generation of pointer variables by eBPF codeGen (3131)
- Fixed bitvec.h (3239)
- P4C-DPDK - Support Non-zero arguments for default action (3206)
- Treat match_kind as a regular type (3099)
- Add support for Hash extern to eBPF backend (3215)
- Implement Meter extern for eBPF backend (3231)
- Keep track of source position for p4-14 switch statement labels (3141)
- Print the current node type name when checkedTo fails (3116)
- Remove incorrect test for recursion (3199)
- Strength reduction should not remove some casts (3226)
- Add support for InternetChecksum extern to eBPF backend (3194)
- Emit semicolon after method call (3093)
- Flatten local struct decl in dpdk (3108)
- Handle bfrt Info for new type (3103)
- Add correct and error test cases for structure-values expressions (3100)
- Allow casts int to int (3220)
- dpdk: Eliminate unused metadata fields (3096)
- Implement PSA for eBPF backend (3139)
- Improve error messages during type checking (3182)
- Struct expressions can be ConstantExpressions (3125)
- adding tests re the driver (1) missing error messages and (2) giving misleading error messages (3198)
- Add support for ActionProfile extern to PSA/eBPF backend (3177)
- Add support for DirectCounter extern to PSA/eBPF backend (3195)
- Implement EBPF deparser (3136)
- test: Update install_fedora_deps.sh (3190)
- Type of a unary expression is not always the same as the input type (3135)
- Allow P4 filenames containing spaces (3242)
- Constant fold casts of serializble enums (3181)
- DPDK Backend: Move learn instructions constant argument to metadata (3163)
- [dpdk] Emit key name in context file as in bfrt and p4info (3205)
- manage lifetimes of Visitor::visited (3168)
- Refactor SetupJoinPoints to make it more extensible
- sfinae-protect operator== StringRef overload (3132)
- typedef with generic types requires type arguments (3174)
- Dismantle Mux expression in dpdk (3087)
- DPDK Backend: Add support for recirculate() extern (3236)
- Fix error message for cast to 'type' (3241)
- Solution for "Non Type_Bits type bool for expression" error (3120)
- Add packet_in.length() and packet_in.advance() methods to a eBPF target (3137)
- Add dpdk version string (3105)
- DPDK Backend: Fix binary operations with 1st operand is not same as dst operand (3209)
- DPDK: Fix missing action definitions in spec file (3109)
- Handle struct expression in ebfp backend (3173)
- P4C-DPDK-Support header field with Slice as a Table key (3151)
- Preserve some types in constant folding. (3094)
- Try to cancel previous Jenkins runs (3230)

Signed-off-by: Radostin Stoyanov <rstoyanov@fedoraproject.org>
@fruffy fruffy added the core Topics concerning the core segments of the compiler (frontend, midend, parser) label Mar 2, 2023
@kfcripps
Copy link
Contributor

I think this has been fixed by #3122.

@MollyDream Please reopen if I am not correct.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This behavior is unintended and should be fixed. core Topics concerning the core segments of the compiler (frontend, midend, parser)
Projects
None yet
Development

No branches or pull requests

4 participants