Fuel const expr operator cost - #14215
Conversation
Operators inside constant expressions - global initializers, element and data segment offsets, element segment expressions - were each charged a hardcoded 1 fuel unit, so `Config::operator_cost` had no effect on any instantiation-time work. Constant expressions are stored as `ConstOp` rather than `wasmparser::Operator`, so add a `const_op_cost` lookup alongside the existing `cost` lookup and use it in `translate_const_expr`. Default costs are unchanged: every `ConstOp` still costs 1 with the default table.
`module_start` synthesizes the call to a module's `(start ...)` function and manually replicates the fuel accounting that `fuel_before_op` performs for `Operator::Call`. That replica used a hardcoded 1 rather than the configured cost, so the `Call` entry of `Config::operator_cost` did not apply to the start call. Look the cost up from the table instead.
`Config::operator_cost` shipped in 48.0.0, so applying it to const-expr operators and the synthesized start call changes the observable behavior of released public API; note it under 49.0.0's `Changed` section. Also pin a second cost-table entry in the const-expr test so a mis-wired `ConstOp` match arm cannot pass, and document where the two flat per-function entry charges in the start-call test come from.
|
@arcusbuilds can you please confirm that you have followed our AI tools policy, including the point about not ever using LLM output directly in a PR description or any other communications on GitHub? (I ask because most folks would not write a paragraph about how they wrote a test that adds 7 and 100. If this is a false positive, my apologies.) Once you have confirmed this and/or made adjustments to conform to our policy, I will then review the PR -- thanks. |
github-actions
Bot
added
wasmtime:api
Label Messager: wasmtime:configIt looks like you are changing Wasmtime's configuration options. Make sure to
DetailsTo modify this label's message, edit the To add new label messages or remove existing label messages, edit the |
|
@cfallin, I have done the adjustment as per the policy. |
| /// | ||
| /// Constant expressions are stored as [`ConstOp`] rather than | ||
| /// `wasmparser::Operator`, so they need their own lookup, but the costs | ||
| /// come from the same table as [`OperatorCostStrategy::cost`]. |
There was a problem hiding this comment.
Rather than doing this, could we translate to Operator and then use the existing cost machinery?
| /// | `$start` function entry | 1 | | ||
| /// | total | 117 | | ||
| /// | ||
| /// With the default table every op costs 1, so the same module totals 6. |
There was a problem hiding this comment.
No need for the excessively verbose (and LLM-produced) comment here -- anyone wanting to know what the test does can look at it.
| /// | ||
| /// The two entry rows are the flat per-function entry charge | ||
| /// (`FuncEnvironment::new`'s `fuel_consumed: 1`, flushed by `fuel_check` on | ||
| /// function entry), not derived from any operator's cost. |
Translate a `ConstOp` back into the `wasmparser::Operator` it was parsed from and charge it through the existing `OperatorCostStrategy::cost`, rather than carrying a second lookup over the same table. That removes the duplicate cost mapping and the default table that had to be kept in sync with `default_operator_cost` by hand. Only `ConstOp::RefNull` cannot reproduce its immediate, since `TypeConvert::convert_heap_type` has no reverse; a placeholder heap type stands in, which is fine because the cost lookup ignores immediates. Also drop the test doc comments and assert the start-call test under the default cost table as well as a custom one.
cfallin
left a comment
cfallin
left a comment
There was a problem hiding this comment.
Almost there! Happy to merge with the below comment addressed.
| /// and has no reverse conversion, so a placeholder heap type stands in for it. | ||
| /// That is fine here because the cost lookup matches on the operator alone and | ||
| /// ignores its immediates. | ||
| fn const_op_as_operator(op: &ConstOp) -> Operator<'static> { |
There was a problem hiding this comment.
Rather than putting this in tunables, could we put it near the definition of ConstOp, and make it a method on the type (e.g. const_op.to_operator())?
2 participants
Closes #14204.
Const-expr operators and the synthesized start call now use the configured operator costs instead of a hardcoded fuel charge of 1. Adds regression tests and a release note.
The issue also reports that variable per-element costs are missing from const-exprs, but those costs are already applied by the shared
array.newtranslation path used by both const-exprs and function bodies. Existing fuel tests confirm this behavior:tests/all/fuel.wast:289.