Reject ternary/lpm/range match on error-typed table keys - #5676
Conversation
These match kinds treat the key as a numeric value or range, which has no meaning for a non-numeric error key. Only exact and optional are allowed. Fixes p4lang#5637. Signed-off-by: aeron-gh <agab0323@gmail.com>
6319691 to
6c57f3f
Compare
|
@ChrisDodd when you get a chance, mind taking a look at this one? It's a small type-checking change (10 lines in typeCheckStmt.cpp; the rest of the diff is the error test and its generated outputs), should be quick to review. Thanks! |
|
This feels like it should be target/arch specific (in the midend/backend) rather than in the target independent frontend. Can a target really not allow ternary matches on error? If the target uses a 1-hot binary representation for error codes internally, a ternary match might actually make sense (allows matching on a set of errors) |
|
@ChrisDodd thanks for looking. This is coming from the Language side, not a target rule. It was raised as p4lang/p4-spec#1389, and the May 2026 LDWG concluded standard P4 should reject it (tracked as #5637), so I put the check next to the existing match-kind type check in the frontend. The spec defines ternary as k & mask == value & mask, and lpm and range likewise need a numeric key. error is opaque and not serializable, so those operations aren't defined on it at the Language level, regardless of target. Your 1-hot case would work, but it depends on an internal representation the Language hides for error, so a source program can't portably rely on it. In standard P4 you'd match specific errors with exact entries, or optional for the value-or-wildcard case. If you'd rather it not be a hard frontend rule, I'm happy to move it to the midend or gate it so a target can opt out. |
I agree that if a target chose to define a specific bit pattern for parsing errors, they would likely want to do the kinds of things that you say (e.g. matching on specific bit positions). But if a target did that, it would have to use a different type than I understand that this means that type |
|
I did a little searching around the files defining the Tofino architectures, and note these things:
|
|
Thanks for digging into that, Andy. That matches what I'd expect: hardware that wants bit-level matching uses bit<16>, so the language error type having no ternary/lpm/range semantics is the right thing to enforce |
|
From LDWG discussion
Will have further discussion in August LDWG meeting. |
The compiler currently accepts a table key of type
errorwith aternary,lpmorrangematch kind. Those match kinds interpret the key as a numeric value, mask or range, which has no meaning for anerrorvalue. Onlyexactandoptional, which compare for equality, make sense for such a key.This adds a check in the type checker that rejects
ternary,lpmandrangeon anerrorkey and tells the user to useexactoroptionalinstead.Tests:
testdata/p4_16_errors/issue5637.p4covers the three rejected match kinds.testdata/p4_16_samples/issue5637_legal_match_kinds.p4confirmsexactandoptionalon anerrorkey still compile.metrics_test_4.p4hadternaryon an error field, so it now usesoptional.This follows the LDWG discussion in p4lang/p4-spec#1389, which also raised the same question for
boolkeys (e.g.psa-bool-ternary-const-entry-bmv2.p4). I leftbooluntouched on purpose: that case is still open in the spec and has an accepted test relying on it, whereas the LDWG conclusion forerrorwas to disallow it. If the spec later decidesbooltoo, it can follow the same pattern.Fixes #5637.