From 47db4efe26873cd23fccda8acd0f6a5ea98b3a62 Mon Sep 17 00:00:00 2001 From: Sai Date: Thu, 30 Jul 2026 09:43:33 -0700 Subject: [PATCH] Make transition() tolerances dynamic and fix its argument list (VAMS-2023) VAMS-2023 Table 4-20 (Mantis 7810) moves the tolerance arguments of the event functions and of transition() from the "constant expression arguments" column to the "dynamic expression arguments" column. For transition() the LRM form is transition ( expr [ , td [ , rise_time [ , fall_time [ , time_tol ] ] ] ] ) OpenVAF got two things wrong here: - the signature list stopped at four arguments, and the arities were off by one from the third signature onwards (`TRANSITION_DELAY_RISET` took two arguments, not three). The full five-argument LRM form was rejected outright with "invalid argument count", and - worse - the four-argument form `transition(x, td, tr, tf)` matched the signature named `..._TOL`, so `fall_time` was const-checked and a run-time fall time was rejected with "constant expressions must not contain variable references". - with the arities corrected, `time_tol` would still have been const-checked. Per Table 4-20 it is now a dynamic expression, so transition() is removed from the const-expression list in body validation. The tolerances that Table 4-20 *keeps* constant - absdelay's `maxdelay`, ddt's and idt/idtmod's `abstol` - are deliberately left alone. Lowering already read `args[2]`/`args[3]` as the rise and fall time, so it needed no change; `td` and `time_tol` do not affect the continuous (first-order lag) realization and are documented as ignored. Also fixes the "too many arguments" diagnostic, which reported `min_args` instead of `max_args` and so said "expected at most 1 arguments" for every over-long call to an operator with optional arguments. Not touched: `cross`, `above`, `timer` and `absdelta`. Their tolerances are already unrestricted because OpenVAF does not resolve the event expression of `@(...)` at all yet, so there is nothing to relax; giving them real signatures is a separate change. Tests: - ui/transition_tolerance.va: all five argument counts accepted, and a run-time rise time, fall time and time tolerance accepted, with no diagnostics. The four- and five-argument cases both failed before. - ui/transition_tolerance_err.log: six arguments still rejected (now with the correct maximum in the message) and ddt/absdelay tolerances still required to be constant. - integration_tests/VAMS2023_TRANSITION_TOL + the OSDI snapshot: end-to-end compile/link/load of a model whose transition rise time, fall time and tolerance are all computed at run time. Co-Authored-By: Claude Opus 5 --- .../vams2023_transition_tol.va | 33 +++++++++++++++++ openvaf/hir_lower/src/expr.rs | 6 ++++ openvaf/hir_ty/src/builtin.rs | 9 +++-- openvaf/hir_ty/src/inference.rs | 7 ++-- openvaf/hir_ty/src/validation/body.rs | 6 ++-- .../osdi/vams2023_transition_tol.snap | 22 ++++++++++++ openvaf/test_data/ui/transition_tolerance.va | 36 +++++++++++++++++++ .../test_data/ui/transition_tolerance_err.log | 24 +++++++++++++ .../test_data/ui/transition_tolerance_err.va | 22 ++++++++++++ 9 files changed, 158 insertions(+), 7 deletions(-) create mode 100644 integration_tests/VAMS2023_TRANSITION_TOL/vams2023_transition_tol.va create mode 100644 openvaf/test_data/osdi/vams2023_transition_tol.snap create mode 100644 openvaf/test_data/ui/transition_tolerance.va create mode 100644 openvaf/test_data/ui/transition_tolerance_err.log create mode 100644 openvaf/test_data/ui/transition_tolerance_err.va diff --git a/integration_tests/VAMS2023_TRANSITION_TOL/vams2023_transition_tol.va b/integration_tests/VAMS2023_TRANSITION_TOL/vams2023_transition_tol.va new file mode 100644 index 00000000..32d44ea9 --- /dev/null +++ b/integration_tests/VAMS2023_TRANSITION_TOL/vams2023_transition_tol.va @@ -0,0 +1,33 @@ +// VAMS-2023 4.5.9 / Table 4-20 (Mantis 7810): every argument of +// +// transition ( expr [ , td [ , rise_time [ , fall_time [ , time_tol ] ] ] ] ) +// +// is a dynamic expression. Earlier revisions required `time_tol` to be a +// constant expression. +`include "disciplines.vams" + +module vams2023_transition_tol(a, c); + inout a, c; + electrical a, c; + + parameter real r = 1000.0 from (0.0:inf); + parameter real base = 1e-9 from (0.0:inf); + parameter real thresh = 0.5; + + real level; + real speed; + real tol; + real drive; + + analog begin + level = (V(a, c) > thresh) ? 1.0 : 0.0; + + // rise time, fall time and time tolerance are all computed at run time + speed = base * (1.0 + r / 1000.0); + tol = speed / 100.0; + + drive = transition(level, 0.0, speed, 2.0 * speed, tol); + + I(a, c) <+ drive * V(a, c) / r; + end +endmodule diff --git a/openvaf/hir_lower/src/expr.rs b/openvaf/hir_lower/src/expr.rs index 83456545..b3bf4239 100644 --- a/openvaf/hir_lower/src/expr.rs +++ b/openvaf/hir_lower/src/expr.rs @@ -874,6 +874,12 @@ impl BodyLoweringCtx<'_, '_, '_> { // time constant is the rise time when the target is increasing and // the fall time when decreasing — a continuous output the solver // integrates through, with the requested transition speed. + // + // `transition(expr, td, rise_time, fall_time, time_tol)`: the + // delay and the (dynamic, VAMS-2023 Table 4-20) `time_tol` do not + // affect this continuous realization and are ignored - `time_tol` + // bounds how precisely a simulator places the time point of the + // transition, which a lag has no notion of. let eps = self.ctx.fconst(1e-12); let rise = if args.len() > 2 { self.lower_expr(args[2]) } else { eps }; let fall = if args.len() > 3 { self.lower_expr(args[3]) } else { rise }; diff --git a/openvaf/hir_ty/src/builtin.rs b/openvaf/hir_ty/src/builtin.rs index 6eeee682..b300eda2 100644 --- a/openvaf/hir_ty/src/builtin.rs +++ b/openvaf/hir_ty/src/builtin.rs @@ -262,12 +262,15 @@ bultins! { } + // VAMS-2023 4.5.9: + // transition ( expr [ , td [ , rise_time [ , fall_time [ , time_tol ] ] ] ] ) + // Every argument is a dynamic expression (Table 4-20, Mantis 7810). TRANSITION = const { fn TRANSITION_NO_ARGS(Val(Real)) -> Real; fn TRANSITION_DELAY(Val(Real),Val(Real)) -> Real; - fn TRANSITION_DELAY_RISET(Val(Real),Val(Real)) -> Real; - fn TRANSITION_DELAY_RISET_FALLT(Val(Real),Val(Real),Val(Real)) -> Real; - fn TRANSITION_DELAY_RISET_FALLT_TOL(Val(Real),Val(Real),Val(Real), Val(Real)) -> Real; + fn TRANSITION_DELAY_RISET(Val(Real),Val(Real),Val(Real)) -> Real; + fn TRANSITION_DELAY_RISET_FALLT(Val(Real),Val(Real),Val(Real),Val(Real)) -> Real; + fn TRANSITION_DELAY_RISET_FALLT_TOL(Val(Real),Val(Real),Val(Real),Val(Real),Val(Real)) -> Real; } diff --git a/openvaf/hir_ty/src/inference.rs b/openvaf/hir_ty/src/inference.rs index 6c8c34c1..b555e3c8 100755 --- a/openvaf/hir_ty/src/inference.rs +++ b/openvaf/hir_ty/src/inference.rs @@ -655,9 +655,12 @@ impl Ctx<'_> { return (default_return_ty(info.signatures), false); } - if info.max_args.map_or(false, |max_args| max_args < args.len()) { + if let Some(max_args) = info.max_args.filter(|&max_args| max_args < args.len()) { + // the "too many arguments" message has to report the maximum, not the + // minimum (which produced "expected at most 1 arguments" for every + // over-long call to an operator with optional arguments) self.result.diagnostics.push(InferenceDiagnostic::ArgCntMismatch { - expected: info.min_args, + expected: max_args, found: args.len(), expr, exact, diff --git a/openvaf/hir_ty/src/validation/body.rs b/openvaf/hir_ty/src/validation/body.rs index 5ed80c18..281fce3a 100644 --- a/openvaf/hir_ty/src/validation/body.rs +++ b/openvaf/hir_ty/src/validation/body.rs @@ -13,7 +13,6 @@ use syntax::name::{AsIdent, Name}; use crate::builtin::{ ABSDELAY_MAX, DDT_TOL, IDT_IC_ASSERT_TOL, NATURE_ACCESS_BRANCH, NATURE_ACCESS_NODES, NATURE_ACCESS_NODE_GND, NATURE_ACCESS_PORT_FLOW, NOISE_TABLE_INLINE, NOISE_TABLE_INLINE_NAME, - TRANSITION_DELAY_RISET_FALLT_TOL, }; use crate::db::HirTyDB; use crate::inference::{BranchWrite, InferenceResult, ResolvedFun}; @@ -724,8 +723,11 @@ impl ExprValidator<'_, '_> { } } + // NOTE: `transition` is deliberately absent. VAMS-2023 Table 4-20 + // (Mantis 7810) lists all of its arguments - including `time_tol` - + // as dynamic expressions; only `absdelay`'s `maxdelay`, `ddt`'s and + // `idt`/`idtmod`'s `abstol` are still constant expressions. (BuiltIn::absdelay, Some(ABSDELAY_MAX)) - | (BuiltIn::transition, Some(TRANSITION_DELAY_RISET_FALLT_TOL)) | (BuiltIn::ddt, Some(DDT_TOL)) | (BuiltIn::idt | BuiltIn::idtmod, Some(IDT_IC_ASSERT_TOL)) => { if let [other_args @ .., const_expr] = args { diff --git a/openvaf/test_data/osdi/vams2023_transition_tol.snap b/openvaf/test_data/osdi/vams2023_transition_tol.snap new file mode 100644 index 00000000..20b8f257 --- /dev/null +++ b/openvaf/test_data/osdi/vams2023_transition_tol.snap @@ -0,0 +1,22 @@ +param "$mfactor" +units = "", desc = "Multiplier (Verilog-A $mfactor)", flags = ParameterFlags(PARA_KIND_INST) +param "r" +units = "", desc = "", flags = ParameterFlags(0x0) +param "base" +units = "", desc = "", flags = ParameterFlags(0x0) +param "thresh" +units = "", desc = "", flags = ParameterFlags(0x0) + +2 terminals +node "a" units = "V", runits = "A" +node "c" units = "V", runits = "A" +node "implicit_equation_0" units = "", runits = "" +jacobian (a, a) JacobianFlags(JACOBIAN_ENTRY_RESIST | JACOBIAN_ENTRY_REACT_CONST) +jacobian (a, c) JacobianFlags(JACOBIAN_ENTRY_RESIST | JACOBIAN_ENTRY_REACT_CONST) +jacobian (a, implicit_equation_0) JacobianFlags(JACOBIAN_ENTRY_RESIST | JACOBIAN_ENTRY_REACT_CONST) +jacobian (c, a) JacobianFlags(JACOBIAN_ENTRY_RESIST | JACOBIAN_ENTRY_REACT_CONST) +jacobian (c, c) JacobianFlags(JACOBIAN_ENTRY_RESIST | JACOBIAN_ENTRY_REACT_CONST) +jacobian (c, implicit_equation_0) JacobianFlags(JACOBIAN_ENTRY_RESIST | JACOBIAN_ENTRY_REACT_CONST) +jacobian (implicit_equation_0, implicit_equation_0) JacobianFlags(JACOBIAN_ENTRY_RESIST | JACOBIAN_ENTRY_REACT | JACOBIAN_ENTRY_REACT_CONST) +0 states +has bound_step false diff --git a/openvaf/test_data/ui/transition_tolerance.va b/openvaf/test_data/ui/transition_tolerance.va new file mode 100644 index 00000000..44d9dcd6 --- /dev/null +++ b/openvaf/test_data/ui/transition_tolerance.va @@ -0,0 +1,36 @@ +// VAMS-2023 4.5.9 / Table 4-20 (Mantis 7810): +// +// transition ( expr [ , td [ , rise_time [ , fall_time [ , time_tol ] ] ] ] ) +// +// All five arguments are dynamic expressions, so none of them has to be a +// constant expression. +module transition_tolerance; + parameter real tr = 1e-9; + parameter real tf = 2e-9; + + real level; + real speed; + real tol; + real y1; + real y2; + real y3; + real y4; + real y5; + + analog begin + level = 1.0; + speed = tr; + tol = tf / 100.0; + + // all argument counts from one to five are accepted + y1 = transition(level); + y2 = transition(level, 0.0); + y3 = transition(level, 0.0, tr); + y4 = transition(level, 0.0, tr, tf); + y5 = transition(level, 0.0, tr, tf, tol); + + // and every argument may be a run-time expression, including the + // fall time and the time tolerance + y1 = transition(level, 0.0, speed, speed * 2.0, speed / 100.0); + end +endmodule diff --git a/openvaf/test_data/ui/transition_tolerance_err.log b/openvaf/test_data/ui/transition_tolerance_err.log new file mode 100644 index 00000000..50bd5cec --- /dev/null +++ b/openvaf/test_data/ui/transition_tolerance_err.log @@ -0,0 +1,24 @@ +error: invalid argument count: expected at most 5 arguments but found 6 + --> /transition_tolerance_err.va:14:13 + | +14 | y = transition(level, 0.0, 1e-9, 2e-9, 1e-12, 1.0); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected at most 5 arguments + +error: constant expressions must not contain variable references + --> /transition_tolerance_err.va:17:24 + | + 6 | real dyn_tol; + | ------- help: 'dyn_tol' was declared here + . +17 | y = ddt(level, dyn_tol); + | ^^^^^^^ not allowed here + +error: constant expressions must not contain variable references + --> /transition_tolerance_err.va:20:35 + | + 6 | real dyn_tol; + | ------- help: 'dyn_tol' was declared here + . +20 | y = absdelay(level, 1e-9, dyn_tol); + | ^^^^^^^ not allowed here + diff --git a/openvaf/test_data/ui/transition_tolerance_err.va b/openvaf/test_data/ui/transition_tolerance_err.va new file mode 100644 index 00000000..e2621376 --- /dev/null +++ b/openvaf/test_data/ui/transition_tolerance_err.va @@ -0,0 +1,22 @@ +// `transition` still takes at most five arguments, and the analog operators +// whose tolerance VAMS-2023 Table 4-20 keeps as a *constant* expression are +// unaffected by Mantis 7810. +module transition_tolerance_err; + real level; + real dyn_tol; + real y; + + analog begin + level = 1.0; + dyn_tol = 1e-12 * level; + + // one argument too many + y = transition(level, 0.0, 1e-9, 2e-9, 1e-12, 1.0); + + // `ddt`'s abstol is still a constant expression argument + y = ddt(level, dyn_tol); + + // ... and so is `absdelay`'s maxdelay + y = absdelay(level, 1e-9, dyn_tol); + end +endmodule