Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new RFC at docs/rfcs/0001-acir-layout-level-specification.md introducing the CAL (Cascode Layout) specification and language. The document defines motivation, goals, design principles, system architecture and toolchain position, a complete language spec (lexical rules and statements like place/anchor/rect/path/via/port/label/group/cell/inst/repeat), coordinate/orientation/layer/net/attribute semantics, PDK workspace database (pdk.db) contract, emission to GDS-II/OASIS, verification integration (DRC/LVS/PEX), error/versioning semantics, examples, and extension points. Also adds multiple D2 diagram resources under docs/rfcs/resources/0001 illustrating stages, data flow, emission, and verification pipelines. Sequence Diagram(s)sequenceDiagram
participant Designer as Designer
participant Cascode_EL as Cascode_EL
participant CAL as CAL
participant GDS as GDS
participant DRC as DRC
participant LVS as LVS
participant PEX as PEX
Designer->>Cascode_EL: provide electrical-level IR
Cascode_EL->>CAL: generate CAL layout document
CAL->>GDS: emit GDS/OASIS
GDS->>DRC: submit for DRC
GDS->>LVS: submit for LVS
GDS->>PEX: submit for PEX/Sim
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@docs/rfcs/0001-acir-layout-level-specification.md`:
- Around line 163-166: Many fenced code blocks in the RFC lack language
specifiers (MD040); update each triple-backtick block (e.g., the block
containing "place M1 nfet_01v8 (W=1u, L=100n, NF=2) @ (5u, 10u) R0") to include
an appropriate language tag (for ACIR-LL use ```acir-ll or ```text, for YAML use
```yaml, for EBNF use ```ebnf, for shell use ```shell or ```bash, for pseudocode
use ```text/```pseudocode) and apply the same change to all other fenced blocks
referenced in the review so they render with correct syntax highlighting.
- Around line 2284-2293: Expand the "10. Security Considerations" section to
include four concrete recommendations: add a "PDK binding integrity" subsection
instructing implementations to verify PDK binding signatures/checksums and to
sandbox or vet P-cell execution; add an "Input validation and DoS protection"
subsection that mandates limits and guards against deeply nested groups/repeat
blocks, huge repeat ranges, circular references, and coordinate-precision
attacks to prevent resource exhaustion; add a "Supply chain security" subsection
recommending secure distribution, signing, and update verification for PDK
bindings; and add an "Audit logging" subsection requiring recording of PDK
versions, P-cell identifiers and execution decisions for reproducibility and
security audits, referencing the existing "PDK binding" terminology so
implementers can locate where to apply these controls.
- Around line 1164-1167: Add a short subsection under Section 3.1.7 (or 3.3.11)
that formally documents the conditional orientation form introduced by the
grammar rule orientation ::= ... | "if" expr "then" orientation "else"
orientation; describe that the conditional accepts a boolean expression (expr)
and list allowed operators and precedence (e.g., arithmetic %, comparison ==,
!=, <, >, logical &&, ||, unary ! with standard precedence and short-circuit
semantics), allow referencing constants and loop variables (e.g., the example if
i % 2 == 0 then R0 else MY), specify evaluation order and that the chosen branch
yields one of the named orientations (R0, R90, R180, R270, MX, MY, MXR90,
MYR90), and note error/undefined behavior rules when expr is non-boolean or
identifiers are undefined.
- Line 2306: The document header and the inline reference "ACIR (RFC-0001)" are
ambiguous—decide a single numbering scheme and make it consistent: either remove
the parenthetical RFC tag from the inline phrase "ACIR (RFC-0001)" if this file
is the canonical RFC-0001 ACIR-LL, or assign a distinct RFC identifier to this
level (e.g., change the header title from RFC-0001 to RFC-0001-LL and update the
inline reference accordingly), or adopt a hierarchical scheme (e.g., change
"ACIR (RFC-0001)" to "ACIR (RFC-0001-LL)" and update the document title/metadata
to match); update all occurrences of "ACIR (RFC-0001)" and the document's RFC
label (document title/metadata) to the chosen scheme so the header and in-text
references (search for "ACIR", "ACIR-LL", and "RFC-0001") are consistent.
- Around line 1753-1764: The error taxonomy table currently groups all
PDK-related problems under E4xxx; split E4xxx into finer categories by adding
E41xx (PDK loading & version errors), E42xx (layer & via definition errors),
E43xx (P-cell reference & parameter errors), and E44xx (anchor definition errors
in PDK), and update the table and any explanatory text referencing E4xxx to
enumerate these new subcategories so tools can map specific PDK failures to
E41xx–E44xx consistently.
- Around line 532-535: The doc states that W is width per finger and PDK
bindings must normalize, but provides no guidance or tests for authors; add to
Section 4 (PDK Binding Specification) a requirement that PDK binding authors
must document the foundry P‑cell W convention and the normalization performed
(explicitly referencing parameters W and NF), add a concrete test case to
Section 7.3 (Expander Requirements) that verifies total width = W * NF for
multi‑finger devices, and add an example in Appendix A showing a foundry P‑cell
using W as total width and the exact normalization steps the PDK binding applies
to convert it to ACIR‑LL convention.
- Around line 437-439: The grid snapping formula currently uses round(value /
grid) without a defined tie-breaking mode; update the spec to replace the
ambiguous use of round() by specifying the exact rounding function (e.g., use
round_half_up(value / grid) * grid) and define round_half_up semantics (ties
rounded away from zero) so snapped_value computation is deterministic; reference
the snapped_value expression and the identifiers value and grid and add the
explicit definition of round_half_up where rounding behavior is described.
- Around line 731-738: The current via placement formula (via_x, via_y using N,
M, cx, cy, px, py) is ambiguous for even-sized arrays; update the spec to state
explicitly that the division (N-1)/2 is floating-point, that the computed
via_x/via_y values represent ideal continuous positions and are then snapped to
the manufacturing/grid coordinate system using a defined snapping rule (e.g.,
round-to-nearest grid step with specified grid resolution or require pitch to be
integer multiples of the grid), and clarify that for even N or M the array is
centered about the geometric center (i.e., vias are offset by half-pitch so the
center lies between vias) rather than shifting all vias by an extra half-pitch;
reference the via_x/via_y expressions and the symbols N, M, cx, cy, px, py when
adding these clarifications.
| ``` | ||
| snapped_value = round(value / grid) * grid | ||
| ``` |
There was a problem hiding this comment.
Specify the rounding mode for grid snapping.
The grid snapping formula uses round() without specifying the rounding mode (e.g., round-half-up, round-half-even/banker's rounding, round-half-down). Different implementations might use different defaults, leading to non-deterministic coordinate quantization on grid boundaries, which violates the design principle of deterministic expansion (Section 1.4, line 80).
Suggested clarification
Grid snapping formula:
-```
-snapped_value = round(value / grid) * grid
-```
+```
+snapped_value = round_half_up(value / grid) * grid
+```
+
+Where `round_half_up` rounds to the nearest integer, with ties (exact halfway cases) rounded away from zero.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ``` | |
| snapped_value = round(value / grid) * grid | |
| ``` | |
| Grid snapping formula: | |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
437-437: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
437-437: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` around lines 437 - 439,
The grid snapping formula currently uses round(value / grid) without a defined
tie-breaking mode; update the spec to replace the ambiguous use of round() by
specifying the exact rounding function (e.g., use round_half_up(value / grid) *
grid) and define round_half_up semantics (ties rounded away from zero) so
snapped_value computation is deterministic; reference the snapped_value
expression and the identifiers value and grid and add the explicit definition of
round_half_up where rounding behavior is described.
| P-cell width convention: | ||
|
|
||
| The `W` parameter specifies width per finger. For multi-finger devices (NF > 1), total device width is `W * NF`. This convention is fixed by ACIR-LL; PDK bindings must normalize to this interpretation. | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding PDK binding validation guidance for the W parameter convention.
The requirement that "PDK bindings must normalize to this interpretation" (line 535) places critical responsibility on PDK binding authors. Since different foundry P-cells may use different W conventions (W as per-finger vs. W as total width), incorrect normalization would cause LVS mismatches.
Consider adding to Section 4 (PDK Binding Specification):
- A validation requirement that PDK binding authors must document how they normalize foundry P-cell parameters to ACIR-LL conventions
- A test case in Section 7.3 (Expander Requirements) that verifies W * NF behavior
- An example in Appendix A showing a P-cell where foundry convention differs and how the PDK binding normalizes it
This would help prevent a class of subtle LVS errors.
🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` around lines 532 - 535,
The doc states that W is width per finger and PDK bindings must normalize, but
provides no guidance or tests for authors; add to Section 4 (PDK Binding
Specification) a requirement that PDK binding authors must document the foundry
P‑cell W convention and the normalization performed (explicitly referencing
parameters W and NF), add a concrete test case to Section 7.3 (Expander
Requirements) that verifies total width = W * NF for multi‑finger devices, and
add an example in Appendix A showing a foundry P‑cell using W as total width and
the exact normalization steps the PDK binding applies to convert it to ACIR‑LL
convention.
| ``` | ||
| For array NxM at center (cx, cy) with pitch (px, py): | ||
| for i in 0 to N-1: | ||
| for j in 0 to M-1: | ||
| via_x = cx + (i - (N-1)/2) * px | ||
| via_y = cy + (j - (M-1)/2) * py | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Clarify via array centering for even-sized arrays.
The formula via_x = cx + (i - (N-1)/2) * px produces non-integer offsets when N is even (e.g., for a 2×1 array, the offsets would be -0.5 and +0.5 pitch). The specification should clarify:
- Whether the division
(N-1)/2uses floating-point arithmetic - How these fractional offsets interact with grid snapping
- Whether the array is truly centered or if there's a half-pitch offset for even arrays
This is critical for predictable via placement in matching structures.
Suggested clarification
Add after line 738:
+
+Note: For even-sized arrays, the geometric center falls between via positions. The formula produces symmetric placement around the specified center coordinate. All via positions are subject to grid snapping after offset calculation.
+
+Example: For a 2x1 array at center (10u, 5u) with pitch (200n, 0):
+ - Via 0: x = 10u + (0 - 0.5) * 200n = 9.9u
+ - Via 1: x = 10u + (1 - 0.5) * 200n = 10.1u🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
731-731: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
731-731: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` around lines 731 - 738,
The current via placement formula (via_x, via_y using N, M, cx, cy, px, py) is
ambiguous for even-sized arrays; update the spec to state explicitly that the
division (N-1)/2 is floating-point, that the computed via_x/via_y values
represent ideal continuous positions and are then snapped to the
manufacturing/grid coordinate system using a defined snapping rule (e.g.,
round-to-nearest grid step with specified grid resolution or require pitch to be
integer multiples of the grid), and clarify that for even N or M the array is
centered about the geometric center (i.e., vias are offset by half-pitch so the
center lies between vias) rather than shifting all vias by an extra half-pitch;
reference the via_x/via_y expressions and the symbols N, M, cx, cy, px, py when
adding these clarifications.
| orientation = "R0" | "R90" | "R180" | "R270" | ||
| | "MX" | "MY" | "MXR90" | "MYR90" | ||
| | "if" expr "then" orientation "else" orientation ; | ||
|
|
There was a problem hiding this comment.
Specify semantics of conditional orientation expressions.
The grammar allows conditional orientation using if <expr> then <orientation> else <orientation> (line 1166), and Section 3.3.11 (line 998) shows an example with if i % 2 == 0 then R0 else MY. However, Section 3.1.7 (Orientations, lines 274-289) doesn't document this conditional syntax or define:
- What types of expressions are valid in the condition (only boolean? comparison operators?)
- Operator precedence and available operators (%, ==, !=, <, >, &&, ||, !)
- Whether the condition can reference constants, loop variables, or other identifiers
Consider adding a subsection to 3.1.7 or 3.3.11 that explicitly defines conditional orientation syntax and semantics.
🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` around lines 1164 - 1167,
Add a short subsection under Section 3.1.7 (or 3.3.11) that formally documents
the conditional orientation form introduced by the grammar rule orientation ::=
... | "if" expr "then" orientation "else" orientation; describe that the
conditional accepts a boolean expression (expr) and list allowed operators and
precedence (e.g., arithmetic %, comparison ==, !=, <, >, logical &&, ||, unary !
with standard precedence and short-circuit semantics), allow referencing
constants and loop variables (e.g., the example if i % 2 == 0 then R0 else MY),
specify evaluation order and that the chosen branch yields one of the named
orientations (R0, R90, R180, R270, MX, MY, MXR90, MYR90), and note
error/undefined behavior rules when expr is non-boolean or identifiers are
undefined.
|
|
||
| | Code | Category | Description | | ||
| |------|----------|-------------| | ||
| | E1xxx | Syntax | Parsing errors | | ||
| | E2xxx | Reference | Undefined identifiers, forward references | | ||
| | E3xxx | Type | Type mismatches, invalid expressions | | ||
| | E4xxx | PDK | PDK loading, version, parameter errors | | ||
| | E5xxx | Semantic | Group validation, net consistency | | ||
| | W1xxx | Grid | Coordinate snapping warnings | | ||
| | W2xxx | Overlap | Geometry overlap warnings | | ||
| | W3xxx | LVS | Potential LVS issues | | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider expanding the error code taxonomy.
The current error code taxonomy has only 5 error categories and 3 warning categories. For a language this complex, consider subdividing E4xxx (PDK) into finer-grained categories:
- E41xx: PDK loading and version errors
- E42xx: Layer and via definition errors
- E43xx: P-cell reference and parameter errors
- E44xx: Anchor definition errors in PDK
This would make error categorization more precise and help tool developers implement better error handling and user feedback.
Enhanced error taxonomy
| Code | Category | Description |
|------|----------|-------------|
| E1xxx | Syntax | Parsing errors |
| E2xxx | Reference | Undefined identifiers, forward references |
| E3xxx | Type | Type mismatches, invalid expressions |
-| E4xxx | PDK | PDK loading, version, parameter errors |
+| E41xx | PDK Load | PDK file loading and version errors |
+| E42xx | PDK Layer | Layer and via definition errors |
+| E43xx | PDK P-cell | P-cell reference and parameter errors |
+| E44xx | PDK Anchor | Anchor definition and evaluation errors |
| E5xxx | Semantic | Group validation, net consistency |
| W1xxx | Grid | Coordinate snapping warnings |
| W2xxx | Overlap | Geometry overlap warnings |
| W3xxx | LVS | Potential LVS issues |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| | Code | Category | Description | | |
| |------|----------|-------------| | |
| | E1xxx | Syntax | Parsing errors | | |
| | E2xxx | Reference | Undefined identifiers, forward references | | |
| | E3xxx | Type | Type mismatches, invalid expressions | | |
| | E4xxx | PDK | PDK loading, version, parameter errors | | |
| | E5xxx | Semantic | Group validation, net consistency | | |
| | W1xxx | Grid | Coordinate snapping warnings | | |
| | W2xxx | Overlap | Geometry overlap warnings | | |
| | W3xxx | LVS | Potential LVS issues | | |
| | Code | Category | Description | | |
| |------|----------|-------------| | |
| | E1xxx | Syntax | Parsing errors | | |
| | E2xxx | Reference | Undefined identifiers, forward references | | |
| | E3xxx | Type | Type mismatches, invalid expressions | | |
| | E41xx | PDK Load | PDK file loading and version errors | | |
| | E42xx | PDK Layer | Layer and via definition errors | | |
| | E43xx | PDK P-cell | P-cell reference and parameter errors | | |
| | E44xx | PDK Anchor | Anchor definition and evaluation errors | | |
| | E5xxx | Semantic | Group validation, net consistency | | |
| | W1xxx | Grid | Coordinate snapping warnings | | |
| | W2xxx | Overlap | Geometry overlap warnings | | |
| | W3xxx | LVS | Potential LVS issues | |
🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` around lines 1753 - 1764,
The error taxonomy table currently groups all PDK-related problems under E4xxx;
split E4xxx into finer categories by adding E41xx (PDK loading & version
errors), E42xx (layer & via definition errors), E43xx (P-cell reference &
parameter errors), and E44xx (anchor definition errors in PDK), and update the
table and any explanatory text referencing E4xxx to enumerate these new
subcategories so tools can map specific PDK failures to E41xx–E44xx
consistently.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@docs/rfcs/0001-acir-layout-level-specification.md`:
- Around line 1119-1121: The grammar for anchor_ref currently permits a
standalone IDENT (anchor_ref, IDENT, hier_ident) which is ambiguous; update the
RFC text near the anchor_ref production to add a short clarifying note that
standalone IDENTs in anchor_ref contexts are resolved first by looking up
user-defined anchors, then by falling back to constants, and specify what
happens on conflict or if neither exists (e.g., error or undefined), so
implementers follow a single resolution order.
- Line 1717: Change the phrase "Coordinate off grid" to the hyphenated form
"Coordinate off-grid" to use a compound modifier consistently; locate the text
matching "Coordinate off grid" in the document (line contains that exact phrase)
and replace it with "Coordinate off-grid".
- Around line 657-660: The spec currently lists "Minimum: 2 vertices" and
"Maximum: Implementation-defined; minimum 1000 vertices" without guidance for
very large paths; update the ACIR layout path vertex section to (1) recommend a
higher practical guidance value (e.g., recommend a suggested maximum or
recommended minimum capacity such as 10,000 vertices) and (2) explicitly
document expected behavior when an implementation encounters a path exceeding
its limit (options: reject with an error, truncate, or split into sub-paths) so
implementations and toolchains have consistent handling; reference and update
the lines that define "Minimum" and "Maximum" and the sentence about "Manhattan
and diagonal segments are permitted" to include the new guidance and behavior
policy.
- Around line 1956-1964: The references inside group dp.M_P use an unnecessary
full path: replace diffpair.dp.M_N.f0@origin and diffpair.dp.M_N.f2@origin with
the shorter relative forms dp.M_N.f0@origin and dp.M_N.f2@origin (or update the
scoping rules/spec text to explicitly allow fully-qualified sibling references)
so that the placements in group dp.M_P (symbols f0, f2, place lines) are
consistent with local reference usage like f0@origin and f2@origin.
- Around line 2455-2460: The P-cell anchors (anchors in the mim_cap P-cell)
reference an implicit parameter "size" derived from capacitance C but that
derivation isn't defined; update the P-cell and spec so implementers know how to
compute it by either adding a computed parameter declaration for size (e.g., in
the P-cell parameters add size with a computed expression derived from C and
cap_density) or by extending Section 4.5 of the PDK binding specification to
define the supported "computed parameter" mechanism and show the formula (e.g.,
size := sqrt(C / cap_density)); ensure the anchors (origin, plus, minus,
bbox.ll, bbox.ur) keep using size after you add the explicit/computed parameter
and reference C and cap_density symbols in the explanation.
|
|
||
| - Minimum: 2 vertices | ||
| - Maximum: Implementation-defined; minimum 1000 vertices | ||
| - Manhattan and diagonal segments are permitted, subject to PDK rules |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding guidance for path vertex count in very large designs.
The vertex constraint states "Maximum: Implementation-defined; minimum 1000 vertices". For scalability in large analog designs (e.g., long spiral inductors), consider specifying a recommended minimum (e.g., 10,000) or documenting the expected behavior when implementations encounter paths exceeding their limit (truncation, error, splitting).
🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` around lines 657 - 660,
The spec currently lists "Minimum: 2 vertices" and "Maximum:
Implementation-defined; minimum 1000 vertices" without guidance for very large
paths; update the ACIR layout path vertex section to (1) recommend a higher
practical guidance value (e.g., recommend a suggested maximum or recommended
minimum capacity such as 10,000 vertices) and (2) explicitly document expected
behavior when an implementation encounters a path exceeding its limit (options:
reject with an error, truncate, or split into sub-paths) so implementations and
toolchains have consistent handling; reference and update the lines that define
"Minimum" and "Maximum" and the sentence about "Manhattan and diagonal segments
are permitted" to include the new guidance and behavior policy.
| anchor_ref = hier_ident "@" IDENT | ||
| | hier_ident "@" IDENT "." ( "x" | "y" ) | ||
| | IDENT ; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify anchor_ref grammar for standalone identifiers.
The anchor_ref production allows a standalone IDENT (line 1121), which could be ambiguous with constants or other identifiers in expression contexts. Consider adding a note clarifying that standalone identifiers in anchor_ref contexts are resolved first as user-defined anchors, then as constants, to establish a clear resolution order.
🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` around lines 1119 - 1121,
The grammar for anchor_ref currently permits a standalone IDENT (anchor_ref,
IDENT, hier_ident) which is ambiguous; update the RFC text near the anchor_ref
production to add a short clarifying note that standalone IDENTs in anchor_ref
contexts are resolved first by looking up user-defined anchors, then by falling
back to constants, and specify what happens on conflict or if neither exists
(e.g., error or undefined), so implementers follow a single resolution order.
| - Type mismatch in expression | ||
|
|
||
| Warnings (report, may continue): | ||
| - Coordinate off grid (snapped automatically) |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: Consider hyphenating "off grid" for consistency.
Per static analysis hint, "off grid" on line 1717 could be hyphenated as "off-grid" when used as a compound modifier ("Coordinate off-grid"). This is a minor style consistency suggestion.
🧰 Tools
🪛 LanguageTool
[grammar] ~1717-~1717: Use a hyphen to join words.
Context: ...(report, may continue): - Coordinate off grid (snapped automatically) - Overlappi...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` at line 1717, Change the
phrase "Coordinate off grid" to the hyphenated form "Coordinate off-grid" to use
a compound modifier consistently; locate the text matching "Coordinate off grid"
in the document (line contains that exact phrase) and replace it with
"Coordinate off-grid".
| group dp.M_P : fingers { | ||
| place f0 nfet_01v8 (W=finger_W, L=finger_L, NF=1) | ||
| @ diffpair.dp.M_N.f0@origin + (finger_pitch, 0) MY | ||
| place f1 nfet_01v8 (W=finger_W, L=finger_L, NF=1) | ||
| @ f0@origin + (finger_pitch, 0) MY | ||
| place f2 nfet_01v8 (W=finger_W, L=finger_L, NF=1) | ||
| @ diffpair.dp.M_N.f2@origin + (finger_pitch, 0) MY | ||
| place f3 nfet_01v8 (W=finger_W, L=finger_L, NF=1) | ||
| @ f2@origin + (finger_pitch, 0) MY |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Potential redundant path prefix in anchor reference.
At line 1958, the reference diffpair.dp.M_N.f0@origin is used from within the diffpair group. Per Section 3.3.8's scoping rules ("Relative references within a group can use short names"), the sibling reference could be simplified to dp.M_N.f0@origin since dp.M_P and dp.M_N are both inside diffpair.
Similarly at line 1962, diffpair.dp.M_N.f2@origin could be dp.M_N.f2@origin.
This is stylistically inconsistent with line 1960-1961 which correctly uses local references (f0@origin). Consider clarifying whether fully-qualified paths are allowed (and simply resolved) or if this is an error.
🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` around lines 1956 - 1964,
The references inside group dp.M_P use an unnecessary full path: replace
diffpair.dp.M_N.f0@origin and diffpair.dp.M_N.f2@origin with the shorter
relative forms dp.M_N.f0@origin and dp.M_N.f2@origin (or update the scoping
rules/spec text to explicitly allow fully-qualified sibling references) so that
the placements in group dp.M_P (symbols f0, f2, place lines) are consistent with
local reference usage like f0@origin and f2@origin.
| anchors: | ||
| origin: { expr: "[0, 0]" } | ||
| plus: { expr: "[size/2, size/2]", note: "size computed from C" } | ||
| minus: { expr: "[size/2, size/2]" } | ||
| bbox.ll: { expr: "[0, 0]" } | ||
| bbox.ur: { expr: "[size, size]" } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify implicit parameter derivation in P-cell anchor expressions.
The mim_cap anchor expressions reference size with a note "size computed from C", but the size parameter is not defined in the parameters section. This implicit derivation from capacitance value should be documented more explicitly in the PDK binding specification (Section 4.5), either as:
- A supported "computed parameter" mechanism, or
- An explicit parameter with a formula (e.g.,
size: { type: real, computed: "sqrt(C / cap_density)" })
This ensures implementers understand how to handle derived parameters.
🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` around lines 2455 - 2460,
The P-cell anchors (anchors in the mim_cap P-cell) reference an implicit
parameter "size" derived from capacitance C but that derivation isn't defined;
update the P-cell and spec so implementers know how to compute it by either
adding a computed parameter declaration for size (e.g., in the P-cell parameters
add size with a computed expression derived from C and cap_density) or by
extending Section 4.5 of the PDK binding specification to define the supported
"computed parameter" mechanism and show the formula (e.g., size := sqrt(C /
cap_density)); ensure the anchors (origin, plus, minus, bbox.ll, bbox.ur) keep
using size after you add the explicit/computed parameter and reference C and
cap_density symbols in the explanation.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/rfcs/0001-acir-layout-level-specification.md`:
- Around line 9-10: The header's Target Version value is inconsistent with the
rest of the document (examples and migration text use 1.1); update the "Target
Version" header value from 1.0 to 1.1 and search/replace all remaining
occurrences of "1.0" that are meant to refer to the spec version so they match
"1.1" (check the "Target Version" header and the migration/example sections that
currently reference 1.1 to ensure uniformity).
- Around line 1135-1155: The grammar references NUMBER in attr_value but NUMBER
is undefined; either add a lexical rule NUMBER = INTEGER | FLOAT (or NUMBER =
INTEGER | FLOAT | PHYSICAL if SI-prefixed quantities are allowed) or change
attr_value to reference existing numeric tokens (e.g., replace NUMBER with
INTEGER | FLOAT or PHYSICAL). Update the grammar so attr_value = "true" |
"false" | STRING | IDENT | NUMBER (with NUMBER defined as above) or directly use
INTEGER/FLOAT/PHYSICAL to remove the undefined symbol.
| | Created | 2026-01-25 | | ||
| | Target Version | ACIR-LL 1.0 | |
There was a problem hiding this comment.
Resolve spec version mismatch (1.0 vs 1.1).
Header says Target Version 1.0, but examples and migration text use 1.1. Pick one version and make it consistent to avoid ambiguous compliance.
🛠️ Suggested fix (align header to 1.1)
-| Target Version | ACIR-LL 1.0 |
+| Target Version | ACIR-LL 1.1 |Also applies to: 356-358, 1735-1738, 2446-2451
🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` around lines 9 - 10, The
header's Target Version value is inconsistent with the rest of the document
(examples and migration text use 1.1); update the "Target Version" header value
from 1.0 to 1.1 and search/replace all remaining occurrences of "1.0" that are
meant to refer to the spec version so they match "1.1" (check the "Target
Version" header and the migration/example sections that currently reference 1.1
to ensure uniformity).
| attributes = "[" attr { "," attr } "]" ; | ||
| attr = IDENT "=" attr_value ; | ||
| attr_value = "true" | "false" | STRING | IDENT | NUMBER ; | ||
|
|
||
| hier_ident = IDENT { "." IDENT } [ "[" IDENT "]" ] ; | ||
|
|
||
| orientation = "R0" | "R90" | "R180" | "R270" | ||
| | "MX" | "MY" | "MXR90" | "MYR90" | ||
| | "if" expr "then" orientation "else" orientation ; | ||
|
|
||
| comment = "//" { ANY_CHAR } NEWLINE ; | ||
|
|
||
| (* Lexical rules *) | ||
| IDENT = LETTER { LETTER | DIGIT | "_" } ; | ||
| INTEGER = [ "-" ] DIGIT { DIGIT } ; | ||
| FLOAT = [ "-" ] DIGIT { DIGIT } "." { DIGIT } [ EXPONENT ] ; | ||
| PHYSICAL = ( INTEGER | FLOAT ) [ SI_PREFIX ] ; | ||
| SI_PREFIX = "T" | "G" | "M" | "k" | "m" | "u" | "n" | "p" | "f" ; | ||
| EXPONENT = ( "e" | "E" ) [ "+" | "-" ] DIGIT { DIGIT } ; | ||
| VERSION = DIGIT { DIGIT } "." DIGIT { DIGIT } [ "." DIGIT { DIGIT } ] ; | ||
| STRING = '"' { ANY_CHAR_EXCEPT_QUOTE } '"' ; |
There was a problem hiding this comment.
Define NUMBER (or replace it) in the grammar.
attr_value references NUMBER, but no lexical rule defines it. This leaves parsers without a formal token definition.
🛠️ Suggested fix
-attr_value = "true" | "false" | STRING | IDENT | NUMBER ;
+attr_value = "true" | "false" | STRING | IDENT | NUMBER ;
@@
+NUMBER = PHYSICAL | FLOAT | INTEGER ;🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` around lines 1135 - 1155,
The grammar references NUMBER in attr_value but NUMBER is undefined; either add
a lexical rule NUMBER = INTEGER | FLOAT (or NUMBER = INTEGER | FLOAT | PHYSICAL
if SI-prefixed quantities are allowed) or change attr_value to reference
existing numeric tokens (e.g., replace NUMBER with INTEGER | FLOAT or PHYSICAL).
Update the grammar so attr_value = "true" | "false" | STRING | IDENT | NUMBER
(with NUMBER defined as above) or directly use INTEGER/FLOAT/PHYSICAL to remove
the undefined symbol.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/rfcs/0001-acir-layout-level-specification.md`:
- Around line 2487-2490: The subsection label "### C.4 Cross-Group Reference" in
the appendix is a numbering typo under Appendix B; update the heading to the
correct label "### B.4 Cross-Group Reference" (replace the "### C.4" token) so
the appendix numbering matches Appendix B, leaving the surrounding text (e.g.,
the line containing "M4@origin = (6.04u + 520n, 10u) = (6.56u, 10u)") unchanged.
| - M4@origin = (6.04u + 520n, 10u) = (6.56u, 10u) | ||
|
|
||
| ### C.4 Cross-Group Reference | ||
|
|
There was a problem hiding this comment.
Fix appendix subsection label (“C.4” under Appendix B).
This appears to be a numbering typo and should be consistent with Appendix B.
🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` around lines 2487 - 2490,
The subsection label "### C.4 Cross-Group Reference" in the appendix is a
numbering typo under Appendix B; update the heading to the correct label "###
B.4 Cross-Group Reference" (replace the "### C.4" token) so the appendix
numbering matches Appendix B, leaving the surrounding text (e.g., the line
containing "M4@origin = (6.04u + 520n, 10u) = (6.56u, 10u)") unchanged.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/rfcs/0001-acir-layout-level-specification.md`:
- Around line 178-202: The document currently only states that geometric
coordinates are in meters and shows SI-prefixed examples (e.g., 10k, 1p) that
could be mistaken for lengths; update the unit semantics by specifying that
every non-geometry parameter must carry a typed unit either via the PDK binding
(parameter type -> base unit mapping) or an explicit base-unit suffix (e.g.,
"10kΩ" or "1pF"), and clarify that the expander interprets values according to
their parameter type (geometry -> meters, resistance -> ohms, capacitance ->
farads) and converts to integer database units accordingly; add a short rule
that bare SI-prefixed numbers without a parameter context are only allowed for
geometry (meters) to avoid ambiguity.
- Around line 389-417: The spec currently allows a grid value not aligned to dbu
which can yield non-integer GDS database units; update the text for the "dbu"
and "grid" declarations to either (a) require that grid is an integer multiple
of dbu (state "grid % dbu == 0") or (b) explicitly define the conversion order
and rounding semantics (e.g., "first snap coordinates to grid, then convert
snapped_value to DBU using dbu conversion and round to nearest integer DBU") so
implementations produce identical integer GDS coordinates; reference the `dbu`
and `grid` declarations and the snapping formula when making the change.
|
|
||
| ``` | ||
| dbu 1n | ||
| ``` | ||
|
|
||
| Optional. Overrides the PDK's default database unit. | ||
|
|
||
| Specifies the physical dimension of one GDS database unit. Common values: | ||
| - `1n` (1 nanometer) - typical for 65nm and older | ||
| - `500p` (0.5 nanometer) - typical for advanced nodes | ||
| - `100p` (0.1 nanometer) - high-precision applications | ||
|
|
||
| If omitted, the PDK binding's default is used. | ||
|
|
||
| #### 3.2.5 Manufacturing Grid Declaration | ||
|
|
||
| ``` | ||
| grid 5n | ||
| ``` | ||
|
|
||
| Optional. Specifies coordinate quantization resolution. | ||
|
|
||
| All coordinates are rounded to the nearest multiple of this value before conversion to database units. If omitted, the PDK binding's default is used. | ||
|
|
||
| Grid snapping formula: | ||
| ``` | ||
| snapped_value = round(value / grid) * grid | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Require grid and DBU compatibility to guarantee integer GDS coordinates.
Grid snapping is defined before DBU conversion, but there is no requirement that grid is an integer multiple of dbu. If not, snapping can yield non-integer DBU values and divergent rounding across implementations. Please mandate compatibility or define the conversion order precisely.
✅ Proposed fix
Optional. Overrides the PDK's default database unit.
Specifies the physical dimension of one GDS database unit.
@@
If omitted, the PDK binding's default is used.
+Compatibility requirement:
+The manufacturing grid MUST be an integer multiple of the DBU (or vice versa)
+so that snapped coordinates convert to integer DBU values. If not, expansion
+MUST fail with a deterministic error.🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
390-390: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
405-405: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
414-414: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
414-414: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` around lines 389 - 417,
The spec currently allows a grid value not aligned to dbu which can yield
non-integer GDS database units; update the text for the "dbu" and "grid"
declarations to either (a) require that grid is an integer multiple of dbu
(state "grid % dbu == 0") or (b) explicitly define the conversion order and
rounding semantics (e.g., "first snap coordinates to grid, then convert
snapped_value to DBU using dbu conversion and round to nearest integer DBU") so
implementations produce identical integer GDS coordinates; reference the `dbu`
and `grid` declarations and the snapping formula when making the change.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/rfcs/0001-acir-layout-level-specification.md`:
- Around line 1613-1630: The inline D2 block in the "6.4 Parasitic Extraction
(PEX)" section won't render as an image in standard Markdown; extract the D2
content (the nodes "inputs", "pex", "annotated", "sim" and the edge "inputs ->
pex -> annotated -> sim") into a separate pex.d2 file, render it to an SVG
(pex.svg), and replace the inline block with a Markdown image reference  so the flow displays consistently like the other sections.
In `@docs/rfcs/resources/0001/emit.d2`:
- Around line 1-17: Add an explicit vars block to the diagram (the pipeline:
"ACIR-LL Emission to GDS" graph and its nodes like doc, parser, consts, anchors,
validate, expand, writer, gds) to set the Tala layout engine for consistent
rendering—mirror the vars/layout settings used in lvs.d2 (e.g., specify layout:
"tala" or equivalent engine key) and place it at the top of emit.d2 so the
entire diagram uses Tala for consistent visuals.
In `@docs/rfcs/resources/0001/lvs.d2`:
- Around line 1-23: Add an explicit vars block specifying the Tala layout engine
so the diagram renders consistently (e.g., add a top-level vars block that names
the layout engine used by the layout_engine node); update the D2 file near the
existing nodes (referencing the layout_engine identifier) to include the vars
entry that sets the engine to "tala" so it matches the other diagrams.
| ### 6.4 Parasitic Extraction (PEX) | ||
|
|
||
| Post-LVS, parasitic extraction derives RC values from layout geometry. | ||
|
|
||
| Flow: | ||
| ```d2 | ||
| direction: right | ||
|
|
||
| inputs: "GDS + SPICE netlist" | ||
| pex: "PEX tool" | ||
| annotated: "Annotated SPICE" | ||
| sim: "Post-layout simulation" | ||
|
|
||
| inputs -> pex -> annotated -> sim | ||
| ``` | ||
|
|
||
| Net attributions enable parasitic-to-net mapping without relying solely on extracted connectivity. | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
PEX flow diagram is embedded inline.
Unlike the DRC (Section 6.1) and LVS (Section 6.2) flows which reference external .svg files, the PEX flow uses an inline D2 code block. This will not render as an image in standard Markdown viewers. Consider extracting to resources/0001/pex.d2 and rendering to SVG for consistency.
Suggested change
-Flow:
-```d2
-direction: right
-
-inputs: "GDS + SPICE netlist"
-pex: "PEX tool"
-annotated: "Annotated SPICE"
-sim: "Post-layout simulation"
-
-inputs -> pex -> annotated -> sim
-```
+Flow:
+Then create docs/rfcs/resources/0001/pex.d2 with the D2 content.
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
1618-1618: Fenced code blocks should be surrounded by blank lines
(MD031, blanks-around-fences)
🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` around lines 1613 - 1630,
The inline D2 block in the "6.4 Parasitic Extraction (PEX)" section won't render
as an image in standard Markdown; extract the D2 content (the nodes "inputs",
"pex", "annotated", "sim" and the edge "inputs -> pex -> annotated -> sim") into
a separate pex.d2 file, render it to an SVG (pex.svg), and replace the inline
block with a Markdown image reference  so the flow displays
consistently like the other sections.
| direction: down | ||
|
|
||
| pipeline: "ACIR-LL Emission to GDS" { | ||
| grid-columns: 1 | ||
| vertical-gap: 16 # try 0–16 | ||
|
|
||
| doc: "ACIR-LL document" {style.fill: "#cfd2dd"} | ||
| parser: "Parser\nlex + syntax + AST" | ||
| consts: "Constant Resolution\nsubstitute + expand repeat" | ||
| anchors: "Anchor Resolution\ncompute + resolve" | ||
| validate: "Validator\nPDK + layer + params + nets + groups" | ||
| expand: "Expander\ninstantiate P-cells + expand vias + units->DBU + snap + emit mode" | ||
| writer: "GDS Writer\nstream formatting + records + output" | ||
| gds: "GDS-II file" {style.fill: "#cfd2dd"} | ||
| } | ||
|
|
||
| pipeline.doc -> pipeline.parser -> pipeline.consts -> pipeline.anchors -> pipeline.validate -> pipeline.expand -> pipeline.writer -> pipeline.gds |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding vars block for layout engine consistency.
Same as lvs.d2, this diagram could benefit from explicit Tala layout engine configuration for rendering consistency.
🤖 Prompt for AI Agents
In `@docs/rfcs/resources/0001/emit.d2` around lines 1 - 17, Add an explicit vars
block to the diagram (the pipeline: "ACIR-LL Emission to GDS" graph and its
nodes like doc, parser, consts, anchors, validate, expand, writer, gds) to set
the Tala layout engine for consistent rendering—mirror the vars/layout settings
used in lvs.d2 (e.g., specify layout: "tala" or equivalent engine key) and place
it at the top of emit.d2 so the entire diagram uses Tala for consistent visuals.
| direction: right | ||
|
|
||
| acir_ll: "ACIR-LL" {style.fill: "#cfd2dd"} | ||
| expander: "Expander" | ||
| gds: "GDS" {style.fill: "#cfd2dd"} | ||
|
|
||
| acir_el: "ACIR-EL" {style.fill: "#7fdbf8"} | ||
| layout_engine: "Layout engine" | ||
| spicegen: "SPICE generator" | ||
| spice_source_netlist: "SPICE source netlist" {style.fill: "#cfd2dd"} | ||
|
|
||
| extractor: "Extractor" | ||
| layout_netlist: "Extracted layout netlist" {style.fill: "#cfd2dd"} | ||
|
|
||
| lvs: "LVS tool" | ||
| results: "Results" {style.fill: "#e4dbff"} | ||
|
|
||
| acir_ll -> expander -> gds | ||
| gds -> extractor -> layout_netlist -> lvs | ||
|
|
||
| acir_el -> layout_engine -> acir_ll | ||
| acir_el -> spicegen -> spice_source_netlist -> lvs | ||
| lvs -> results |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding vars block for layout engine consistency.
Unlike drc.d2 and data-flow.d2, this diagram is missing the vars block specifying the Tala layout engine. While D2 may have a default, explicitly specifying it ensures consistent rendering across all diagrams.
Suggested addition
+vars: {
+ d2-config: {
+ layout-engine: tala
+ }
+}
+
direction: right📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| direction: right | |
| acir_ll: "ACIR-LL" {style.fill: "#cfd2dd"} | |
| expander: "Expander" | |
| gds: "GDS" {style.fill: "#cfd2dd"} | |
| acir_el: "ACIR-EL" {style.fill: "#7fdbf8"} | |
| layout_engine: "Layout engine" | |
| spicegen: "SPICE generator" | |
| spice_source_netlist: "SPICE source netlist" {style.fill: "#cfd2dd"} | |
| extractor: "Extractor" | |
| layout_netlist: "Extracted layout netlist" {style.fill: "#cfd2dd"} | |
| lvs: "LVS tool" | |
| results: "Results" {style.fill: "#e4dbff"} | |
| acir_ll -> expander -> gds | |
| gds -> extractor -> layout_netlist -> lvs | |
| acir_el -> layout_engine -> acir_ll | |
| acir_el -> spicegen -> spice_source_netlist -> lvs | |
| lvs -> results | |
| vars: { | |
| d2-config: { | |
| layout-engine: tala | |
| } | |
| } | |
| direction: right | |
| acir_ll: "ACIR-LL" {style.fill: "#cfd2dd"} | |
| expander: "Expander" | |
| gds: "GDS" {style.fill: "#cfd2dd"} | |
| acir_el: "ACIR-EL" {style.fill: "#7fdbf8"} | |
| layout_engine: "Layout engine" | |
| spicegen: "SPICE generator" | |
| spice_source_netlist: "SPICE source netlist" {style.fill: "#cfd2dd"} | |
| extractor: "Extractor" | |
| layout_netlist: "Extracted layout netlist" {style.fill: "#cfd2dd"} | |
| lvs: "LVS tool" | |
| results: "Results" {style.fill: "#e4dbff"} | |
| acir_ll -> expander -> gds | |
| gds -> extractor -> layout_netlist -> lvs | |
| acir_el -> layout_engine -> acir_ll | |
| acir_el -> spicegen -> spice_source_netlist -> lvs | |
| lvs -> results |
🤖 Prompt for AI Agents
In `@docs/rfcs/resources/0001/lvs.d2` around lines 1 - 23, Add an explicit vars
block specifying the Tala layout engine so the diagram renders consistently
(e.g., add a top-level vars block that names the layout engine used by the
layout_engine node); update the D2 file near the existing nodes (referencing the
layout_engine identifier) to include the vars entry that sets the engine to
"tala" so it matches the other diagrams.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@docs/rfcs/resources/0001/data-flow.d2`:
- Line 3: The pipeline title string "ACIR Data Flow: EL to Layout" is misleading
given the diagram shows the full flow through to GDS-II/OASIS; update the
pipeline declaration (the string in the pipeline: "..." - e.g., pipeline: "ACIR
Data Flow: EL to Layout") to a more accurate title such as "ACIR Data Flow: EL
to Layout (GDS-II/OASIS)" or "ACIR Data Flow: EL through GDS-II/OASIS" so the
header matches the diagram's scope.
| @@ -0,0 +1,14 @@ | |||
| direction: down | |||
|
|
|||
| pipeline: "ACIR Data Flow: EL to Layout" { | |||
There was a problem hiding this comment.
Pipeline title is misleading.
The title says "EL to Layout" but the diagram shows the complete flow from ACIR-EL through to GDS-II/OASIS. Consider updating to reflect the full scope.
📝 Suggested fix
-pipeline: "ACIR Data Flow: EL to Layout" {
+pipeline: "ACIR Data Flow: EL to GDS" {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pipeline: "ACIR Data Flow: EL to Layout" { | |
| pipeline: "ACIR Data Flow: EL to GDS" { |
🤖 Prompt for AI Agents
In `@docs/rfcs/resources/0001/data-flow.d2` at line 3, The pipeline title string
"ACIR Data Flow: EL to Layout" is misleading given the diagram shows the full
flow through to GDS-II/OASIS; update the pipeline declaration (the string in the
pipeline: "..." - e.g., pipeline: "ACIR Data Flow: EL to Layout") to a more
accurate title such as "ACIR Data Flow: EL to Layout (GDS-II/OASIS)" or "ACIR
Data Flow: EL through GDS-II/OASIS" so the header matches the diagram's scope.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@docs/rfcs/0001-acir-layout-level-specification.md`:
- Around line 1815-1846: Add a short clarifying note in the ACIR differential
pair examples explaining that the identifiers M_N and M_P denote the
differential signal side (negative vs positive) and not transistor type, and
that both M_N and M_P may be NMOS (e.g., nfet_01v8) or PMOS depending on the
circuit; update the example comments near the group declarations (diffpair,
group M_N, group M_P) to state this convention explicitly and ensure any other
diff-pair examples in the same document use the same wording for consistency.
- Around line 42-73: Condense the long enumerations under headings "1.2 Goals"
and "1.3 Non-Goals" into concise professional prose or a compact table: merge
related bullets (e.g., deterministic output, traceability, iteration,
reviewability, manufacturability, scalability) into one or two summary sentences
under "1.2 Goals" and similarly compress the six "Non-Goals" into a short
paragraph or a two-column table that groups related non-goals (pattern encoding,
constraint solving, routing abstraction, DRC guarantees, methodology, PDK
portability); remove unnecessary bolding and preserve the existing technical
phrases (like "Provide complete geometric specification", "Maintain
traceability", etc.) so reviewers can still find the original points while
complying with the docs style guidance.
| ### 1.2 Goals | ||
|
|
||
| CAL is designed to: | ||
|
|
||
| 1. Provide complete geometric specification. Every construct expands deterministically to GDS-II/OASIS primitives. Given a CAL document and the PDK workspace database (`pdk.db`), the output is bit-identical across runs. | ||
|
|
||
| 2. Maintain traceability. Every geometric element traces to a Cascode EL device or net (or is explicitly marked as layout-only), enabling debugging, verification, and design review. | ||
|
|
||
| 3. Support practical iteration. Anchor-based relative positioning enables local changes without global coordinate recalculation. Constants and expressions reduce repetition and enable single-point-of-change modifications. | ||
|
|
||
| 4. Enable human review. An engineer can read CAL and understand what geometry will be generated without executing expansion tools or opening a layout viewer. | ||
|
|
||
| 5. Produce manufacturing-ready output. Emit GDS-II or OASIS files suitable for foundry submission. | ||
|
|
||
| 6. Scale to complex designs. Grouping constructs, parameterization, and repetition mechanisms support designs with hundreds or thousands of devices. | ||
|
|
||
| ### 1.3 Non-Goals | ||
|
|
||
| CAL explicitly does not: | ||
|
|
||
| 1. Encode layout patterns. Common-centroid, interdigitation, guard rings, and other analog layout techniques are not language constructs. CAL expresses their geometric realization; the layout engine decides which patterns to apply. | ||
|
|
||
| 2. Provide constraint satisfaction. CAL describes geometry, not constraints. Constraints live in EL Cascode circuits or external specifications. | ||
|
|
||
| 3. Abstract routing infrastructure. Track-based routing, pin access grids, and routing channels are not language constructs. CAL draws paths at explicit (absolute or relative) coordinates. | ||
|
|
||
| 4. Guarantee DRC cleanliness. CAL can express DRC-violating geometry. Verification is performed externally. | ||
|
|
||
| 5. Prescribe layout methodology. The language is agnostic to how layout decisions are made, whether by human designers, automated tools, or any other means. | ||
|
|
||
| 6. Provide PDK portability. A CAL document is specific to one PDK. The layout engine that generates CAL may be portable, but the output is PDK-specific due to P-cell geometries and layer mappings. | ||
|
|
There was a problem hiding this comment.
Reduce long bullet lists in Goals/Non‑Goals to match doc style guidance.
These sections rely on two long enumerations; consider condensing into prose or a table to avoid excessive bulleting.
As per coding guidelines: “docs/**/*.md: Spec and documentation writing style: avoid excessive bulleted lists and bolded text. Use professional prose at the appropriate verbosity level. Reserve bold formatting for technical terms being defined, critical warnings, or table headers requiring emphasis”.
🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` around lines 42 - 73,
Condense the long enumerations under headings "1.2 Goals" and "1.3 Non-Goals"
into concise professional prose or a compact table: merge related bullets (e.g.,
deterministic output, traceability, iteration, reviewability, manufacturability,
scalability) into one or two summary sentences under "1.2 Goals" and similarly
compress the six "Non-Goals" into a short paragraph or a two-column table that
groups related non-goals (pattern encoding, constraint solving, routing
abstraction, DRC guarantees, methodology, PDK portability); remove unnecessary
bolding and preserve the existing technical phrases (like "Provide complete
geometric specification", "Maintain traceability", etc.) so reviewers can still
find the original points while complying with the docs style guidance.
| ### 8.2 Grouped Differential Pair | ||
|
|
||
| ``` | ||
| CAL 1.0 | ||
| pdk sky130 1.0.45 | ||
| topcell diff_pair | ||
|
|
||
| // Layout constants | ||
| const finger_pitch = 520n | ||
| const finger_W = 500n | ||
| const finger_L = 180n | ||
| const dp_y = 10u | ||
| const dp_base_x = 5u | ||
|
|
||
| // Differential pair with ABBA interleaving | ||
| group diffpair : match { | ||
|
|
||
| // M_N fingers at A positions (0, 3) | ||
| group M_N : fingers { | ||
| place f0 nfet_01v8 (W=finger_W, L=finger_L, NF=1) | ||
| @ (dp_base_x, dp_y) R0 | ||
| place f1 nfet_01v8 (W=finger_W, L=finger_L, NF=1) | ||
| @ f0@origin + (3 * finger_pitch, 0) R0 | ||
| } | ||
|
|
||
| // M_P fingers at B positions (1, 2) | ||
| group M_P : fingers { | ||
| place f0 nfet_01v8 (W=finger_W, L=finger_L, NF=1) | ||
| @ diffpair.M_N.f0@origin + (finger_pitch, 0) MY | ||
| place f1 nfet_01v8 (W=finger_W, L=finger_L, NF=1) | ||
| @ f0@origin + (finger_pitch, 0) MY | ||
| } |
There was a problem hiding this comment.
Clarify M_N/M_P naming convention in diff‑pair examples.
The examples label devices as M_N/M_P but don’t note that this refers to signal polarity, not transistor type; both can be NMOS in an NMOS diff pair.
Based on learnings: “In documentation files (e.g., docs/rfcs/...) that discuss ACIR differential pair examples, clarify device naming: M_N and M_P refer to the differential signal side they connect to, not the transistor type. Both M_N and M_P can be NMOS devices in an NMOS differential pair. Use this convention consistently to prevent misinterpretation.”
🧰 Tools
🪛 markdownlint-cli2 (0.20.0)
[warning] 1817-1817: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
In `@docs/rfcs/0001-acir-layout-level-specification.md` around lines 1815 - 1846,
Add a short clarifying note in the ACIR differential pair examples explaining
that the identifiers M_N and M_P denote the differential signal side (negative
vs positive) and not transistor type, and that both M_N and M_P may be NMOS
(e.g., nfet_01v8) or PMOS depending on the circuit; update the example comments
near the group declarations (diffpair, group M_N, group M_P) to state this
convention explicitly and ensure any other diff-pair examples in the same
document use the same wording for consistency.
1 participant
Introduces RFC-0001 for ACIR-LL (Analog Circuit Intermediate Representation — Layout Level): a layout-geometry language that stays close to GDS-II/OASIS while remaining readable, deterministic, and traceable back to ACIR-EL devices and nets.
The RFC establishes the problem framing, goals and non-goals, and the intended position of ACIR-LL in the Cascode toolchain (ACIR-HL -> ACIR-ML -> ACIR-EL -> ACIR-LL -> GDS/OASIS).
Motivation / problem statement
The RFC motivates ACIR-LL as a middle ground between manufacturing formats (dense, low-semantic GDS/OASIS) and constraint-only representations (intent without committed geometry). The goal is a representation of committed physical geometry that is reviewable and maintains correspondence to the source design.