Skip to content

Commit d39f38e

Browse files
AaronWebsterAaronWebster
committed
Statically-known existence conditions: drop or simplify the check
Many Emboss schemas have fields whose existence_condition the front end has already constant-folded to a boolean literal: * \`if false:\` fields are never present, so Ok() needs no check for them at all. PR 241 still emitted the full has_X() guard + Ok() check; both branches are dead. * Unconditional fields (and \`if true:\` fields) carry a literal \`true\` existence. has_X() is always Known and always true, so the standard ok_method_test pattern if (!has_x().Known()) return false; if (has_x().ValueOrDefault() && !x().Ok()) return false; collapses to a bare if (!x().Ok()) return false; A fast pre-pass in _generate_optimized_ok_method_body recognizes both shapes (existence_condition.type.boolean has a literal value) and routes them to the short emit paths. \`if false:\` fields are skipped; the always-true case emits the bare Ok() check. Wide golden churn: most structs have several unconditional fields (virtual size fields like IntrinsicSizeInBytes/Max/Min plus any non-conditional payload), so most Ok() methods shrink by several lines apiece. The docstring on _generate_optimized_ok_method_body is rewritten to describe the four emit paths the function now routes fields through (skipped / unconditional / switch arm / ok_method_test) — the original PR 241 docstring referenced only two paths and a duplicate- case fallthrough that no longer exists.
1 parent d3c6e13 commit d39f38e

32 files changed

Lines changed: 981 additions & 2624 deletions

‎compiler/back_end/cpp/header_generator.py‎

Lines changed: 70 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1557,68 +1557,53 @@ def _extract_switch_arms(expression, ir):
15571557
def _generate_optimized_ok_method_body(
15581558
fields, ir, subexpressions, allow_tail_form=False
15591559
):
1560-
"""Generates optimized C++ code for the Ok() method body.
1561-
1562-
This function optimizes validation logic for structures with conditional
1563-
fields by grouping fields that share the same discriminant into switch
1564-
statements, rather than generating separate if-statements for each field.
1565-
1566-
For example, given this Emboss definition:
1567-
1568-
struct Foo:
1569-
0 [+4] UInt tag
1570-
if tag == 0:
1571-
4 [+4] UInt var_0
1572-
if tag == 1:
1573-
4 [+4] UInt var_1
1574-
if tag == 0:
1575-
8 [+4] UInt var_x
1576-
1577-
Instead of generating separate if-statements:
1578-
1579-
if (tag() == 0 && !var_0().Ok()) return false;
1580-
if (tag() == 1 && !var_1().Ok()) return false;
1581-
if (tag() == 0 && !var_x().Ok()) return false;
1582-
1583-
This generates an optimized switch statement:
1584-
1585-
const auto emboss_reserved_switch_discrim = tag();
1586-
if (!emboss_reserved_switch_discrim.Known()) return false;
1587-
switch (emboss_reserved_switch_discrim.ValueOrDefault()) {
1588-
case 0:
1589-
if (!var_0().Ok()) return false;
1590-
break;
1591-
case 1:
1592-
if (!var_1().Ok()) return false;
1593-
break;
1594-
}
1595-
1596-
Note: When multiple fields share the same case value (like var_0 and var_x
1597-
both guarded by tag == 0), only the first field for that case value is
1598-
checked in the switch. This is because once we've checked var_0 for case 0,
1599-
we break out of the switch. Fields with duplicate case values will fall
1600-
through to the if-statement path.
1601-
1602-
Fields with non-equality conditions (or conditions that can't be converted
1603-
to switch cases) are grouped by their rendered condition and emitted as
1604-
if-statements.
1605-
1606-
Additionally, when a condition is statically known to be true (e.g., fields
1607-
that are always present), the if-block wrapper is omitted entirely and the
1608-
field checks are emitted directly. This avoids generating unnecessary
1609-
runtime checks like:
1610-
1611-
const auto emboss_reserved_cond = Maybe<bool>(true);
1612-
if (!emboss_reserved_cond.Known()) return false; // Always true
1613-
if (emboss_reserved_cond.ValueOrDefault()) { ... } // Always true
1560+
"""Generates the field-validation body of a structure's Ok() method.
1561+
1562+
For each conditional field, routes it to one of four emit paths:
1563+
1564+
1. **Skipped**. `if false:` fields can never be present, so Ok()
1565+
needs no check for them at all.
1566+
2. **Unconditional**. Non-conditional and `if true:` fields skip
1567+
the has_${field}() wrapper and emit a bare
1568+
`if (!field().Ok()) return false;` — has_X() is statically
1569+
Known and always true.
1570+
3. **Switch arm**. Fields whose existence condition decomposes
1571+
into a discriminant test (`discrim == K`, a disjunction of
1572+
such tests, or an AND with an equality conjunct) join a
1573+
switch on the shared discriminant. _extract_switch_arms does
1574+
the decomposition; the optimizer then sorts cases by value,
1575+
coalesces arms with identical bodies into multi-label arms,
1576+
and (for the switch that ends the function) rewrites
1577+
single-field bare arms as tail-form `return field().Ok();`.
1578+
A demotion pass falls back to (4) for switch groups whose
1579+
total entry count is below 2 (the switch wrapper is bigger
1580+
than the dedupe saves).
1581+
4. **ok_method_test**. The fallback: the existing
1582+
has_${field}()-based check. Used for conditions that don't
1583+
match any of the above and for switch groups demoted by (3).
1584+
1585+
Each surviving switch group emits a single `switch` statement
1586+
sharing one read of the discriminant subexpression across all the
1587+
fields it guards — the original PR 241 win. The other emit paths
1588+
are post-PR 241 refinements that further shrink the generated
1589+
Ok() on tagged-union and mixed-conditional schemas.
16141590
16151591
Arguments:
16161592
fields: List of field IR nodes to generate Ok() checks for.
16171593
ir: The full IR for looking up references.
1618-
subexpressions: An ExpressionScope for sharing common subexpressions.
1594+
subexpressions: An ExpressionScope for sharing common
1595+
subexpressions across the whole Ok() body.
1596+
allow_tail_form: When True, the very last surviving switch
1597+
group is eligible for tail-form arm rewriting (each
1598+
single-field bare arm becomes `return field().Ok();`
1599+
instead of `if (!X().Ok()) return false; break;`). The
1600+
caller passes True only when the switch is the function's
1601+
tail — i.e., there is no [requires] clause emitted after
1602+
the field-ok-checks.
16191603
16201604
Returns:
1621-
A string containing the C++ code for the optimized Ok() method body.
1605+
A string containing the C++ code for the optimized Ok() method
1606+
body.
16221607
"""
16231608
groups = {}
16241609
ordered_keys = []
@@ -1630,6 +1615,28 @@ def _generate_optimized_ok_method_body(
16301615
field_group_key = {}
16311616

16321617
for field in fields:
1618+
# Fields whose existence condition is statically known take fast
1619+
# paths bypassing both the switch and ok_method_test machinery:
1620+
# * `if false:` — the field is never present, so there's nothing
1621+
# to validate. Drop it entirely.
1622+
# * `if true:` (and non-conditional fields, which carry the
1623+
# boolean-true literal) — has_${field}() is always Known and
1624+
# true, so the has_X()-based guard is dead. Emit just the
1625+
# direct `if (!${field}().Ok()) return false;` check.
1626+
# All unconditional fields share one group so they emit
1627+
# contiguously (at the position of the first such field), which
1628+
# keeps the C++ compiler's view of the Ok() body tight enough to
1629+
# consolidate redundant frame setup across the checks.
1630+
cond = field.existence_condition
1631+
if cond.type.which_type == "boolean" and cond.type.boolean.has_field("value"):
1632+
if cond.type.boolean.value:
1633+
key = "UNCOND"
1634+
if key not in groups:
1635+
groups[key] = {"type": "unconditional", "fields": []}
1636+
ordered_keys.append(key)
1637+
groups[key]["fields"].append(field)
1638+
# else: existence is statically false — emit nothing.
1639+
continue
16331640
discrim_expr, arms = _extract_switch_arms(field.existence_condition, ir)
16341641
if discrim_expr:
16351642
# Render the discriminant unscoped to build the grouping key.
@@ -1736,6 +1743,13 @@ def _generate_optimized_ok_method_body(
17361743
field=_cpp_field_name(field.name.name.text),
17371744
)
17381745
)
1746+
elif group["type"] == "unconditional":
1747+
for field in group["fields"]:
1748+
blocks.append(
1749+
" if (!{}().Ok()) return false;\n\n".format(
1750+
_cpp_field_name(field.name.name.text)
1751+
)
1752+
)
17391753
else:
17401754
for field in group["fields"]:
17411755
blocks.append(

‎testdata/golden_cpp/alignments.emb.h‎

Lines changed: 23 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -75,66 +75,33 @@ class GenericAlignmentsView final {
7575
bool Ok() const {
7676
if (!IsComplete()) return false;
7777

78-
if (!has_zero_offset().Known()) return false;
79-
if (has_zero_offset().ValueOrDefault() && !zero_offset().Ok()) return false;
78+
if (!zero_offset().Ok()) return false;
8079

81-
if (!has_zero_offset_substructure().Known()) return false;
82-
if (has_zero_offset_substructure().ValueOrDefault() &&
83-
!zero_offset_substructure().Ok())
84-
return false;
80+
if (!zero_offset_substructure().Ok()) return false;
8581

86-
if (!has_two_offset_substructure().Known()) return false;
87-
if (has_two_offset_substructure().ValueOrDefault() &&
88-
!two_offset_substructure().Ok())
89-
return false;
82+
if (!two_offset_substructure().Ok()) return false;
9083

91-
if (!has_three_offset().Known()) return false;
92-
if (has_three_offset().ValueOrDefault() && !three_offset().Ok())
93-
return false;
84+
if (!three_offset().Ok()) return false;
9485

95-
if (!has_four_offset().Known()) return false;
96-
if (has_four_offset().ValueOrDefault() && !four_offset().Ok()) return false;
86+
if (!four_offset().Ok()) return false;
9787

98-
if (!has_eleven_offset().Known()) return false;
99-
if (has_eleven_offset().ValueOrDefault() && !eleven_offset().Ok())
100-
return false;
88+
if (!eleven_offset().Ok()) return false;
10189

102-
if (!has_twelve_offset().Known()) return false;
103-
if (has_twelve_offset().ValueOrDefault() && !twelve_offset().Ok())
104-
return false;
90+
if (!twelve_offset().Ok()) return false;
10591

106-
if (!has_zero_offset_four_stride_array().Known()) return false;
107-
if (has_zero_offset_four_stride_array().ValueOrDefault() &&
108-
!zero_offset_four_stride_array().Ok())
109-
return false;
92+
if (!zero_offset_four_stride_array().Ok()) return false;
11093

111-
if (!has_zero_offset_six_stride_array().Known()) return false;
112-
if (has_zero_offset_six_stride_array().ValueOrDefault() &&
113-
!zero_offset_six_stride_array().Ok())
114-
return false;
94+
if (!zero_offset_six_stride_array().Ok()) return false;
11595

116-
if (!has_three_offset_four_stride_array().Known()) return false;
117-
if (has_three_offset_four_stride_array().ValueOrDefault() &&
118-
!three_offset_four_stride_array().Ok())
119-
return false;
96+
if (!three_offset_four_stride_array().Ok()) return false;
12097

121-
if (!has_four_offset_six_stride_array().Known()) return false;
122-
if (has_four_offset_six_stride_array().ValueOrDefault() &&
123-
!four_offset_six_stride_array().Ok())
124-
return false;
98+
if (!four_offset_six_stride_array().Ok()) return false;
12599

126-
if (!has_IntrinsicSizeInBytes().Known()) return false;
127-
if (has_IntrinsicSizeInBytes().ValueOrDefault() &&
128-
!IntrinsicSizeInBytes().Ok())
129-
return false;
100+
if (!IntrinsicSizeInBytes().Ok()) return false;
130101

131-
if (!has_MaxSizeInBytes().Known()) return false;
132-
if (has_MaxSizeInBytes().ValueOrDefault() && !MaxSizeInBytes().Ok())
133-
return false;
102+
if (!MaxSizeInBytes().Ok()) return false;
134103

135-
if (!has_MinSizeInBytes().Known()) return false;
136-
if (has_MinSizeInBytes().ValueOrDefault() && !MinSizeInBytes().Ok())
137-
return false;
104+
if (!MinSizeInBytes().Ok()) return false;
138105

139106
return true;
140107
}
@@ -1335,21 +1302,13 @@ class GenericPlaceholder4View final {
13351302
bool Ok() const {
13361303
if (!IsComplete()) return false;
13371304

1338-
if (!has_dummy().Known()) return false;
1339-
if (has_dummy().ValueOrDefault() && !dummy().Ok()) return false;
1305+
if (!dummy().Ok()) return false;
13401306

1341-
if (!has_IntrinsicSizeInBytes().Known()) return false;
1342-
if (has_IntrinsicSizeInBytes().ValueOrDefault() &&
1343-
!IntrinsicSizeInBytes().Ok())
1344-
return false;
1307+
if (!IntrinsicSizeInBytes().Ok()) return false;
13451308

1346-
if (!has_MaxSizeInBytes().Known()) return false;
1347-
if (has_MaxSizeInBytes().ValueOrDefault() && !MaxSizeInBytes().Ok())
1348-
return false;
1309+
if (!MaxSizeInBytes().Ok()) return false;
13491310

1350-
if (!has_MinSizeInBytes().Known()) return false;
1351-
if (has_MinSizeInBytes().ValueOrDefault() && !MinSizeInBytes().Ok())
1352-
return false;
1311+
if (!MinSizeInBytes().Ok()) return false;
13531312

13541313
return true;
13551314
}
@@ -1738,24 +1697,15 @@ class GenericPlaceholder6View final {
17381697
bool Ok() const {
17391698
if (!IsComplete()) return false;
17401699

1741-
if (!has_zero_offset().Known()) return false;
1742-
if (has_zero_offset().ValueOrDefault() && !zero_offset().Ok()) return false;
1700+
if (!zero_offset().Ok()) return false;
17431701

1744-
if (!has_two_offset().Known()) return false;
1745-
if (has_two_offset().ValueOrDefault() && !two_offset().Ok()) return false;
1702+
if (!two_offset().Ok()) return false;
17461703

1747-
if (!has_IntrinsicSizeInBytes().Known()) return false;
1748-
if (has_IntrinsicSizeInBytes().ValueOrDefault() &&
1749-
!IntrinsicSizeInBytes().Ok())
1750-
return false;
1704+
if (!IntrinsicSizeInBytes().Ok()) return false;
17511705

1752-
if (!has_MaxSizeInBytes().Known()) return false;
1753-
if (has_MaxSizeInBytes().ValueOrDefault() && !MaxSizeInBytes().Ok())
1754-
return false;
1706+
if (!MaxSizeInBytes().Ok()) return false;
17551707

1756-
if (!has_MinSizeInBytes().Known()) return false;
1757-
if (has_MinSizeInBytes().ValueOrDefault() && !MinSizeInBytes().Ok())
1758-
return false;
1708+
if (!MinSizeInBytes().Ok()) return false;
17591709

17601710
return true;
17611711
}

‎testdata/golden_cpp/anonymous_bits.emb.h‎

Lines changed: 16 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -91,27 +91,17 @@ class GenericEmbossReservedAnonymousField2View final {
9191
bool Ok() const {
9292
if (!IsComplete()) return false;
9393

94-
if (!has_high_bit().Known()) return false;
95-
if (has_high_bit().ValueOrDefault() && !high_bit().Ok()) return false;
94+
if (!high_bit().Ok()) return false;
9695

97-
if (!has_bar().Known()) return false;
98-
if (has_bar().ValueOrDefault() && !bar().Ok()) return false;
96+
if (!bar().Ok()) return false;
9997

100-
if (!has_first_bit().Known()) return false;
101-
if (has_first_bit().ValueOrDefault() && !first_bit().Ok()) return false;
98+
if (!first_bit().Ok()) return false;
10299

103-
if (!has_IntrinsicSizeInBits().Known()) return false;
104-
if (has_IntrinsicSizeInBits().ValueOrDefault() &&
105-
!IntrinsicSizeInBits().Ok())
106-
return false;
100+
if (!IntrinsicSizeInBits().Ok()) return false;
107101

108-
if (!has_MaxSizeInBits().Known()) return false;
109-
if (has_MaxSizeInBits().ValueOrDefault() && !MaxSizeInBits().Ok())
110-
return false;
102+
if (!MaxSizeInBits().Ok()) return false;
111103

112-
if (!has_MinSizeInBits().Known()) return false;
113-
if (has_MinSizeInBits().ValueOrDefault() && !MinSizeInBits().Ok())
114-
return false;
104+
if (!MinSizeInBits().Ok()) return false;
115105

116106
return true;
117107
}
@@ -739,24 +729,15 @@ class GenericEmbossReservedAnonymousField1View final {
739729
bool Ok() const {
740730
if (!IsComplete()) return false;
741731

742-
if (!has_bit_23().Known()) return false;
743-
if (has_bit_23().ValueOrDefault() && !bit_23().Ok()) return false;
732+
if (!bit_23().Ok()) return false;
744733

745-
if (!has_low_bit().Known()) return false;
746-
if (has_low_bit().ValueOrDefault() && !low_bit().Ok()) return false;
734+
if (!low_bit().Ok()) return false;
747735

748-
if (!has_IntrinsicSizeInBits().Known()) return false;
749-
if (has_IntrinsicSizeInBits().ValueOrDefault() &&
750-
!IntrinsicSizeInBits().Ok())
751-
return false;
736+
if (!IntrinsicSizeInBits().Ok()) return false;
752737

753-
if (!has_MaxSizeInBits().Known()) return false;
754-
if (has_MaxSizeInBits().ValueOrDefault() && !MaxSizeInBits().Ok())
755-
return false;
738+
if (!MaxSizeInBits().Ok()) return false;
756739

757-
if (!has_MinSizeInBits().Known()) return false;
758-
if (has_MinSizeInBits().ValueOrDefault() && !MinSizeInBits().Ok())
759-
return false;
740+
if (!MinSizeInBits().Ok()) return false;
760741

761742
return true;
762743
}
@@ -1216,28 +1197,15 @@ class GenericFooView final {
12161197
bool Ok() const {
12171198
if (!IsComplete()) return false;
12181199

1219-
if (!has_emboss_reserved_anonymous_field_2().Known()) return false;
1220-
if (has_emboss_reserved_anonymous_field_2().ValueOrDefault() &&
1221-
!emboss_reserved_anonymous_field_2().Ok())
1222-
return false;
1200+
if (!emboss_reserved_anonymous_field_2().Ok()) return false;
12231201

1224-
if (!has_emboss_reserved_anonymous_field_1().Known()) return false;
1225-
if (has_emboss_reserved_anonymous_field_1().ValueOrDefault() &&
1226-
!emboss_reserved_anonymous_field_1().Ok())
1227-
return false;
1202+
if (!emboss_reserved_anonymous_field_1().Ok()) return false;
12281203

1229-
if (!has_IntrinsicSizeInBytes().Known()) return false;
1230-
if (has_IntrinsicSizeInBytes().ValueOrDefault() &&
1231-
!IntrinsicSizeInBytes().Ok())
1232-
return false;
1204+
if (!IntrinsicSizeInBytes().Ok()) return false;
12331205

1234-
if (!has_MaxSizeInBytes().Known()) return false;
1235-
if (has_MaxSizeInBytes().ValueOrDefault() && !MaxSizeInBytes().Ok())
1236-
return false;
1206+
if (!MaxSizeInBytes().Ok()) return false;
12371207

1238-
if (!has_MinSizeInBytes().Known()) return false;
1239-
if (has_MinSizeInBytes().ValueOrDefault() && !MinSizeInBytes().Ok())
1240-
return false;
1208+
if (!MinSizeInBytes().Ok()) return false;
12411209

12421210
if (!has_high_bit().Known()) return false;
12431211
if (has_high_bit().ValueOrDefault() && !high_bit().Ok()) return false;

0 commit comments

Comments
 (0)