Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions changelog.d/7887-pget-guard-branches.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
**`perf(codegen)`: the generic property-get guard branches instead of AND-ing eight predicates — `interp` 0.843 → 0.784 s, `iso_miss` 1.231 → 1.178 s.**

The inline monomorphic-IC diamond built one flat `hit` predicate out of eight tests, so LLVM
if-converted the whole region: every receiver executed every load and every compare —
including the two epoch loads — even after the first test had already decided the answer. It
also tested the two *rare* receiver tags (SSO, INT32 class-ref) before the pointer tag,
putting two constant materialisations, two compares and two branches in front of every real
object read.

The chain now branches — `pget.recv_ok` → `pic.recv_hdr` → `pic.token` → `pic.hit` — and the
POINTER/STRING tag test goes first, with SSO/INT32 discriminated in a cold `pget.recv_other`
(the three tag classes are pairwise disjoint, so the order is free). The four header
predicates stay flat on purpose: they are four loads from the same two cache lines and LLVM
fuses their compares into one `ccmp` chain, which beats four branches. `pic.miss` recomputes
what the polymorphic-way compares need from the same memory rather than taking phis, which
would have dragged their `cset`/`csinc` materialisation back onto the hot path.
Comment on lines +10 to +16

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Check the changelog fragment key against the PR and the referenced issue.
set -euo pipefail

gh pr view 7887 --json number,title,state,headRefName 2>/dev/null || echo "7887 not a PR"
gh issue view 7883 --json number,title,state 2>/dev/null || echo "7883 not an issue"
git log --oneline -5 -- changelog.d/7887-pget-guard-branches.md

Repository: PerryTS/perry

Length of output: 143


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- fragment history ---'
git log --oneline -8 -- changelog.d/7887-pget-guard-branches.md

printf '%s\n' '--- fragment metadata ---'
git status --short -- changelog.d/7887-pget-guard-branches.md
git show --format='commit=%H%nsubject=%s%nparents=%P' --no-patch -- changelog.d/7887-pget-guard-branches.md

printf '%s\n' '--- local references ---'
git branch --show-current
git branch -a --contains HEAD | head -20

printf '%s\n' '--- GitHub metadata ---'
curl --fail --silent --show-error --max-time 20 \
  -H 'Accept: application/vnd.github+json' \
  https://api.github.com/repos/PerryTS/perry/pulls/7887 \
  | jq '{number,title,state,head:{ref,sha},base:{ref,sha},merged_at}'
curl --fail --silent --show-error --max-time 20 \
  -H 'Accept: application/vnd.github+json' \
  https://api.github.com/repos/PerryTS/perry/issues/7883 \
  | jq '{number,title,state,pull_request}'

Repository: PerryTS/perry

Length of output: 709


🌐 Web query:

PerryTS/perry pull request 7887 issue 7883 changelog.d/7887-pget-guard-branches.md

💡 Result:

The query refers to the PerryTS/perry repository, a native TypeScript compiler written in Rust [1]. Based on the provided search results, there is no pull request 7887 or issue 7883 in the PerryTS/perry repository [1][2][3]. The repository's issue numbering in the provided records does not reach 7887; recent issues are in the hundreds or low thousands (e.g., #5779, #5594) [4][5]. Furthermore, no file named changelog.d/7887-pget-guard-branches.md exists in the project; the repository typically uses a single CHANGELOG.md file in the root directory [6]. The terms "pget" and "guard branches" do not appear in the context of the PerryTS/perry project. While "guard" appears in other projects (e.g., in the context of "branch-guard" tools for AI agents [7][8][9] or LLVM loop analysis [10]), these are unrelated to Perry. Perry does have infrastructure for API stub elimination and parity validation, but it does not utilize a mechanism identified by the name "pget-guard-branches" [11][12][13].

Citations:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- remote configuration ---'
git remote -v

printf '%s\n' '--- commit identity and trailers ---'
git show -s --format='commit=%H%nauthor=%an <%ae>%nauthor-date=%aI%ncommitter=%cn <%ce>%ncommit-date=%cI%nsubject=%s%nbody=%b' a2c8fd026a008d74fbeeb12b7b7dc9bf5bac25dd

printf '%s\n' '--- commit file list ---'
git diff-tree --no-commit-id --name-status -r a2c8fd026a008d74fbeeb12b7b7dc9bf5bac25dd

printf '%s\n' '--- issue references in changed code ---'
git show --format= --find-renames a2c8fd026a008d74fbeeb12b7b7dc9bf5bac25dd -- \
  | rg -n -C 2 '`#7883`|generic_dispatch|7887-pget-guard-branches' || true

printf '%s\n' '--- fragment content ---'
cat -n changelog.d/7887-pget-guard-branches.md

Repository: PerryTS/perry

Length of output: 8816


🏁 Script executed:

#!/bin/bash
set -u

for path in pull/7887 issue/7883 commit/a2c8fd026a008d74fbeeb12b7b7dc9bf5bac25dd; do
  printf '\n--- %s ---\n' "$path"
  curl --location --silent --show-error --max-time 45 \
    -H 'User-Agent: review-check' \
    -H 'Accept: text/html' \
    -w '\nHTTP_STATUS=%{http_code}\n' \
    "https://github.com/PerryTS/perry/$path" \
    | tr '\n' ' ' \
    | sed -E 's/<[^>]+>/ /g; s/[[:space:]]+/ /g' \
    | cut -c1-1200
done

Repository: PerryTS/perry

Length of output: 3850


🏁 Script executed:

#!/bin/bash
set -u

for path in pull/7887 issue/7883 commit/a2c8fd026a008d74fbeeb12b7b7dc9bf5bac25dd; do
  body=$(mktemp)
  status=$(curl --location --silent --show-error --max-time 45 \
    -H 'User-Agent: review-check' \
    -o "$body" -w '%{http_code}' \
    "https://github.com/PerryTS/perry/$path")
  printf '\n--- %s (HTTP %s) ---\n' "$path" "$status"
  rg -o -i '.{0,100}(Page not found|not found|<title>[^<]+|canonical[^>]+|pull request #[0-9]+|issue #[0-9]+).{0,180}' \
    "$body" | head -20 || true
  rm -f "$body"
done

Repository: PerryTS/perry

Length of output: 2815


🏁 Script executed:

#!/bin/bash
set -u

for path in pull/7883 discussions/7883; do
  body=$(mktemp)
  status=$(curl --location --silent --show-error --max-time 45 \
    -H 'User-Agent: review-check' \
    -o "$body" -w '%{http_code}' \
    "https://github.com/PerryTS/perry/$path")
  printf '\n--- %s (HTTP %s) ---\n' "$path" "$status"
  rg -o -i '<title>[^<]+|og:url" content="[^"]+|Page not found|Not Found|Pull Request #[0-9]+|Issue #[0-9]+' \
    "$body" | head -12 || true
  rm -f "$body"
done

printf '\n--- PR 7887 head and base markers ---\n'
body=$(mktemp)
curl --location --silent --show-error --max-time 45 \
  -H 'User-Agent: review-check' -o "$body" \
  https://github.com/PerryTS/perry/pull/7887
rg -o -i '.{0,80}(a2c8fd026a008d74fbeeb12b7b7dc9bf5bac25dd|merged|Open|closed).{0,120}' \
  "$body" | head -20 || true
rm -f "$body"

Repository: PerryTS/perry

Length of output: 4412


Update the code-comment references to #7887. The fragment matches PR #7887; #7883 is an unrelated pull request, not the intended issue. The fragment was added with this PR and is not a rebase artifact.

🧰 Tools
🪛 LanguageTool

[style] ~13-~13: Try using a descriptive adverb here.
Context: ...). The four header predicates stay flat on purpose: they are four loads from the same two ...

(ON_PURPOSE_DELIBERATELY)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@changelog.d/7887-pget-guard-branches.md` around lines 10 - 16, Update the
code-comment references associated with this changelog fragment to use issue/PR
reference `#7887` instead of `#7883`, without changing the described branching or
predicate behavior.

Source: Learnings


Semantics are unchanged — every predicate is still checked, with control flow instead of data
flow. `evalNode`'s monomorphic hit path drops from 58 to 46 aarch64 instructions before the
field load.

Measured on the quiet M1 mini, best-of-5, exit-checked, 19-program corpus, both arms linking
the same runtime archives: `interp` −7.0%, `iso_miss` −4.3%, `cycles` −2.8%, `pipeline`
−1.9%, `churn_read` −1.8%, `deeplist`/`retain_wide1` −1.3%. Six binaries compile
byte-identical and set the run's noise floor at ±0.6%.

The guard test walks the def chain rather than asserting presence: it walks the CFG backwards
from the block performing the raw slot load, requires every edge on that path to be the
**true** edge of a `cond_br`, and takes the transitive def closure of those conditions,
requiring each guard to be reachable from a branch condition. Three sabotages were run and all
three went red (constant branch condition, swapped `cond_br` edges, deleted predicate) — a
presence assertion catches none of the first two. The walk must be scoped to a single
function: register names restart at `%r1` in every body, so a global def map resolved the
receiver-tag condition to an unrelated `ptrtoint` in another function.
182 changes: 133 additions & 49 deletions crates/perry-codegen/src/expr/property_get/generic_dispatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,21 +128,19 @@ pub(crate) fn lower_generic_property_get(
// `js_object_get_field_by_name_f64` runtime entry (which
// handles `.length` directly from the NaN-box length
// byte and returns `undefined` for other keys).
let is_sso = ctx.block().icmp_eq(I64, &obj_tag, "32761"); // 0x7FF9
// v0.5.747: INT32-tagged class refs (top16 == 0x7FFE) used
// as PropertyGet receivers. Pre-fix these fell through to
// the invalid-recv path (returning undefined) because the
// 0xFFFD-masked tag check (0x7FFE & 0xFFFD = 0x7FFC, not
// 0x7FFD) treated them as non-pointer values. Drizzle's
// `is(value, type)` chain depends on `Cls.kind` reads through
// an Any-typed local. Refs #420 / #618 followup.
//
// Note: this also catches plain int32 numeric values (e.g.
// `(42).property`). The runtime helper's INT32-tag arm at
// js_object_get_field_by_name returns undefined for any
// class_id not registered in CLASS_DYNAMIC_PROPS, matching
// the previous behavior — pure ints have no static fields.
let is_int32_class = ctx.block().icmp_eq(I64, &obj_tag, "32766"); // 0x7FFE
// v0.5.747: INT32-tagged class refs (top16 == 0x7FFE) used
// as PropertyGet receivers. Pre-fix these fell through to
// the invalid-recv path (returning undefined) because the
// 0xFFFD-masked tag check (0x7FFE & 0xFFFD = 0x7FFC, not
// 0x7FFD) treated them as non-pointer values. Drizzle's
// `is(value, type)` chain depends on `Cls.kind` reads through
// an Any-typed local. Refs #420 / #618 followup.
//
// Note: this also catches plain int32 numeric values (e.g.
// `(42).property`). The runtime helper's INT32-tag arm at
// js_object_get_field_by_name returns undefined for any
// class_id not registered in CLASS_DYNAMIC_PROPS, matching
// the previous behavior — pure ints have no static fields.
let obj_tag_masked = ctx.block().and(I64, &obj_tag, "65533"); // 0xFFFD
let is_valid = ctx.block().icmp_eq(I64, &obj_tag_masked, "32765"); // 0x7FFD
let sso_idx = ctx.new_block("pget.recv_sso");
Expand All @@ -155,19 +153,28 @@ pub(crate) fn lower_generic_property_get(
let invalid_label = ctx.block_label(invalid_idx);
let class_ref_label = ctx.block_label(class_ref_idx);
let final_merge_label = ctx.block_label(final_merge_idx);
// Three-step branch: first check SSO, then class-ref, then
// pointer-validity. Inverse branches funnel into invalid_idx.
let pic_or_invalid_idx = ctx.new_block("pget.check_ptr");
let pic_or_invalid_label = ctx.block_label(pic_or_invalid_idx);
// #7883: the POINTER/STRING test goes FIRST, and the two rare tags are
// discriminated in a cold block off its false edge. The three tag classes
// are pairwise disjoint — `is_valid` is `(tag & 0xFFFD) == 0x7FFD`, true
// only for 0x7FFD/0x7FFF, while SSO is 0x7FF9 and an INT32 class ref is
// 0x7FFE — so testing them in any order gives the same routing. The old
// order (SSO, then class-ref, then pointer) put two 16-bit constant
// materialisations, two compares and two branches in front of every real
// object receiver: 13 instructions before the PIC on the path that is
// taken essentially always. Now it is `lshr` + `and` + `cmp` + branch.
let other_idx = ctx.new_block("pget.recv_other");
let other_label = ctx.block_label(other_idx);
let check_class_ref_idx = ctx.new_block("pget.check_class_ref");
let check_class_ref_label = ctx.block_label(check_class_ref_idx);
ctx.block().cond_br(&is_valid, &pic_label, &other_label);
ctx.current_block = other_idx;
let is_sso = ctx.block().icmp_eq(I64, &obj_tag, "32761"); // 0x7FF9
ctx.block()
.cond_br(&is_sso, &sso_label, &check_class_ref_label);
ctx.current_block = check_class_ref_idx;
let is_int32_class = ctx.block().icmp_eq(I64, &obj_tag, "32766"); // 0x7FFE
ctx.block()
.cond_br(&is_int32_class, &class_ref_label, &pic_or_invalid_label);
ctx.current_block = pic_or_invalid_idx;
ctx.block().cond_br(&is_valid, &pic_label, &invalid_label);
.cond_br(&is_int32_class, &class_ref_label, &invalid_label);

// Class-ref dispatch: route through the runtime helper which
// detects INT32 class-ref bits and consults CLASS_DYNAMIC_PROPS
Expand Down Expand Up @@ -265,29 +272,48 @@ pub(crate) fn lower_generic_property_get(
//
// Threshold matches `js_native_call_method`'s small-handle
// detection (raw_ptr < 0x100000).
let cache_ref = format!("@{}", cache_name);
let is_real_ptr = ctx.block().icmp_ugt(I64, &obj_handle, "1048575"); // 0x100000

// Sentinel address: the per-site cache global itself —
// always valid, 16-byte aligned, and its bytes don't
// match GC_TYPE_OBJECT (=2) or an active keys_array, so
// the IC will cleanly miss when we substitute it for a
// small handle.
let cache_ref = format!("@{}", cache_name);
let cache_addr = ctx.block().ptrtoint(&cache_ref, I64);
let safe_obj_handle = ctx
.block()
.select(I1, &is_real_ptr, I64, &obj_handle, &cache_addr);
// #7883: the hit/miss/merge blocks are minted here so the guard chain
// below can BRANCH OUT to the miss on the first failing predicate
// instead of AND-ing eight of them into one flat `hit`. LLVM if-converts
// a flat predicate, so every receiver paid every load and every compare —
// including the two epoch loads — even after the very first one had
// already decided the answer. Each group now ends in its own `cond_br`;
// the miss block reconstructs what the polymorphic-way compares need
// through phis (`false`/`0` on the early-exit edges, which is exactly
// what the flat predicate computed there).
let hit_idx = ctx.new_block("pic.hit");
let miss_idx = ctx.new_block("pic.miss");
let merge_idx = ctx.new_block("pic.merge");
let hit_label = ctx.block_label(hit_idx);
let miss_label = ctx.block_label(miss_idx);
let merge_label = ctx.block_label(merge_idx);
let hdr_idx = ctx.new_block("pic.recv_hdr");
let hdr_label = ctx.block_label(hdr_idx);
let tok_idx = ctx.new_block("pic.token");
let tok_label = ctx.block_label(tok_idx);
// Small-handle receivers (native-module registry ids) must never be
// dereferenced. Pre-#7883 they were kept out of the loads by selecting a
// sentinel address and AND-ing `is_real_ptr` into `hit`; the branch does
// the same job without putting a `select` (and the sentinel's address
// materialisation) in front of every real object read. The miss path
// still substitutes the sentinel, because the way compares below load
// `field_count` unconditionally.
// (edge labels are no longer needed: the miss block recomputes.)
ctx.block().cond_br(&is_real_ptr, &hdr_label, &miss_label);
ctx.current_block = hdr_idx;

// GcHeader sits 8 bytes before the user pointer; obj_type is the
// first u8 (GC_TYPE_OBJECT=2). Cost: 1 sub + 1 load i8 + 1 cmp
// i8 + 1 and i1 — the cond_br's `is_object` operand is folded
// into the existing branch instruction by LLVM. Branch-predicted
// taken since real PropertyGet receivers are objects.
let gc_type_addr = ctx.block().sub(I64, &safe_obj_handle, "8");
let gc_type_addr = ctx.block().sub(I64, &obj_handle, "8");
let gc_type_ptr = ctx.block().inttoptr(I64, &gc_type_addr);
let gc_type = ctx.block().load(I8, &gc_type_ptr);
let gc_type_ok = ctx.block().icmp_eq(I8, &gc_type, "2");
let is_object = ctx.block().and(I1, &is_real_ptr, &gc_type_ok);
let is_object = ctx.block().icmp_eq(I8, &gc_type, "2");

// Issue #618: closures share GC_TYPE_OBJECT but their offset+16
// is a capture slot, not `keys_array`. The PIC's keys_val ==
Expand All @@ -299,7 +325,7 @@ pub(crate) fn lower_generic_property_get(
// `js_object_get_field_ic_miss` → `js_object_get_field_by_name`,
// which dispatches closure dynamic-prop reads via the
// `CLOSURE_DYNAMIC_PROPS` side-table.
let magic_addr = ctx.block().add(I64, &safe_obj_handle, "12");
let magic_addr = ctx.block().add(I64, &obj_handle, "12");
let magic_ptr = ctx.block().inttoptr(I64, &magic_addr);
let magic_val = ctx.block().load(I32, &magic_ptr);
// CLOSURE_MAGIC = 0x434C4F53 (4 bytes "CLOS" little-endian).
Expand All @@ -321,7 +347,7 @@ pub(crate) fn lower_generic_property_get(
// Specific repro: `function f(): any { ... return new
// RegExp(...) } const r = f(); r.source` — fast path returns
// garbage f64 instead of routing through `js_regexp_get_source`.
let object_type_ptr = ctx.block().inttoptr(I64, &safe_obj_handle);
let object_type_ptr = ctx.block().inttoptr(I64, &obj_handle);
let object_type = ctx.block().load(I32, &object_type_ptr);
let object_type_ok = ctx.block().icmp_eq(I32, &object_type, "1");
let is_object = ctx.block().and(I1, &is_object, &object_type_ok);
Expand All @@ -338,15 +364,25 @@ pub(crate) fn lower_generic_property_get(
// descriptors) whenever it is set. Mirrors the guard in
// `class_field_inline_guard.rs`. Cost: 1 sub + load i16 + and + cmp,
// folded into the existing `hit` cond_br.
let reserved_addr = ctx.block().sub(I64, &safe_obj_handle, "6");
let reserved_addr = ctx.block().sub(I64, &obj_handle, "6");
let reserved_ptr = ctx.block().inttoptr(I64, &reserved_addr);
let reserved = ctx.block().load(crate::types::I16, &reserved_ptr);
let has_desc = ctx.block().and(crate::types::I16, &reserved, "2048"); // OBJ_FLAG_HAS_DESCRIPTORS (0x800)
let no_desc = ctx.block().icmp_eq(crate::types::I16, &has_desc, "0");
let is_object = ctx.block().and(I1, &is_object, &no_desc);

// #7883: first exit. The four header predicates above are kept as one
// flat `and` on purpose — they are four loads from the SAME two cache
// lines and LLVM fuses their compares into a `ccmp` chain, which is
// cheaper than four branches. What was NOT worth folding is everything
// below: the keys load, the token select and the two epoch loads all
// hang off the same predicate, so a non-object receiver used to execute
// them before the flat `hit` could reject it.
ctx.block().cond_br(&is_object, &tok_label, &miss_label);
ctx.current_block = tok_idx;

// Load obj->keys_array at offset 16 of ObjectHeader.
let keys_addr = ctx.block().add(I64, &safe_obj_handle, "16");
let keys_addr = ctx.block().add(I64, &obj_handle, "16");
let keys_ptr_p = ctx.block().inttoptr(I64, &keys_addr);
let keys_val = ctx.block().load(I64, &keys_ptr_p);

Expand All @@ -361,7 +397,7 @@ pub(crate) fn lower_generic_property_get(
// two token kinds can never collide numerically — one compare, no
// discriminant word. `parent_class_id` is a u32 at offset 8 on every
// target (the four leading u32s precede the pointer fields).
let pcid_addr = ctx.block().add(I64, &safe_obj_handle, "8");
let pcid_addr = ctx.block().add(I64, &obj_handle, "8");
let pcid_ptr = ctx.block().inttoptr(I64, &pcid_addr);
let pcid = ctx.block().load(I32, &pcid_ptr);
// In-range test via wrapping add + ult: (pcid - 0x8000_0000) < 0x4000_0000.
Expand Down Expand Up @@ -389,8 +425,7 @@ pub(crate) fn lower_generic_property_get(
// (which resolves inherited props correctly). Id tokens always
// carry bit 62, so they are never zero.
let token_nonnull = ctx.block().icmp_ne(I64, &token, "0");
let hit_token = ctx.block().and(I1, &is_object, &token_eq);
let hit = ctx.block().and(I1, &hit_token, &token_nonnull);
let hit = ctx.block().and(I1, &token_eq, &token_nonnull);

// #6080a: pointer tokens are only trustworthy within the GC epoch they
// were primed in. The `@perry_ic_N` global is invisible to every GC
Expand All @@ -412,12 +447,6 @@ pub(crate) fn lower_generic_property_get(
let epoch_ok = ctx.block().or(I1, &is_stamp, &epoch_eq);
let hit = ctx.block().and(I1, &hit, &epoch_ok);

let hit_idx = ctx.new_block("pic.hit");
let miss_idx = ctx.new_block("pic.miss");
let merge_idx = ctx.new_block("pic.merge");
let hit_label = ctx.block_label(hit_idx);
let miss_label = ctx.block_label(miss_idx);
let merge_label = ctx.block_label(merge_idx);
ctx.block().cond_br(&hit, &hit_label, &miss_label);

// PIC hit: bounds-check the cached slot, then direct field load.
Expand All @@ -434,7 +463,7 @@ pub(crate) fn lower_generic_property_get(
// rule; an out-of-bounds slot falls to the miss path, which reads
// the overflow map correctly (and records the guard failure —
// `record_guard_pass` only fires after the bounds check passes).
let fc_addr = ctx.block().add(I64, &safe_obj_handle, "12");
let fc_addr = ctx.block().add(I64, &obj_handle, "12");
let fc_ptr = ctx.block().inttoptr(I64, &fc_addr);
let fc = ctx.block().load(I32, &fc_ptr);
let fc64 = ctx.block().zext(I32, &fc, I64);
Expand Down Expand Up @@ -486,6 +515,61 @@ pub(crate) fn lower_generic_property_get(
// it was a real miss — the feedback heuristics see an unchanged signal
// (the site IS polymorphic; only the cost of that changed).
ctx.current_block = miss_idx;
// #7883: the guard chain now branches out at three points, so the values
// the polymorphic way compares consult are no longer live on every edge
// into this block — and phi-ing them would drag their materialisation
// (`cset`/`csinc` per value) back onto the hot path, which is the whole
// point of branching. They are recomputed here instead, from the SAME
// memory with no intervening store, so every one is bit-identical to
// what the pre-#7883 flat predicate computed. This block is cold — every
// path out of it either loads a way slot or calls the miss handler.
//
// The small-handle sentinel substitution lives here for the same reason:
// the way compares load `field_count` unconditionally, and a native
// registry-id receiver reaches this block without ever being a pointer.
let cache_addr = ctx.block().ptrtoint(&cache_ref, I64);
let safe_obj_handle = ctx
.block()
.select(I1, &is_real_ptr, I64, &obj_handle, &cache_addr);
let m_gc_type_addr = ctx.block().sub(I64, &safe_obj_handle, "8");
let m_gc_type_ptr = ctx.block().inttoptr(I64, &m_gc_type_addr);
let m_gc_type = ctx.block().load(I8, &m_gc_type_ptr);
let m_gc_type_ok = ctx.block().icmp_eq(I8, &m_gc_type, "2");
let is_object = ctx.block().and(I1, &is_real_ptr, &m_gc_type_ok);
let m_magic_addr = ctx.block().add(I64, &safe_obj_handle, "12");
let m_magic_ptr = ctx.block().inttoptr(I64, &m_magic_addr);
let m_magic = ctx.block().load(I32, &m_magic_ptr);
let m_is_closure = ctx.block().icmp_eq(I32, &m_magic, "1129268819");
let m_not_closure = ctx.block().xor(I1, &m_is_closure, "true");
let is_object = ctx.block().and(I1, &is_object, &m_not_closure);
let m_ot_ptr = ctx.block().inttoptr(I64, &safe_obj_handle);
let m_ot = ctx.block().load(I32, &m_ot_ptr);
let m_ot_ok = ctx.block().icmp_eq(I32, &m_ot, "1");
let is_object = ctx.block().and(I1, &is_object, &m_ot_ok);
let m_res_addr = ctx.block().sub(I64, &safe_obj_handle, "6");
let m_res_ptr = ctx.block().inttoptr(I64, &m_res_addr);
let m_res = ctx.block().load(crate::types::I16, &m_res_ptr);
let m_has_desc = ctx.block().and(crate::types::I16, &m_res, "2048");
let m_no_desc = ctx.block().icmp_eq(crate::types::I16, &m_has_desc, "0");
let is_object = ctx.block().and(I1, &is_object, &m_no_desc);
Comment on lines +530 to +554

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Compare small-handle sentinel substitution across inline-guard lowerings.
set -euo pipefail

rg -n -C 6 'safe_obj_handle|is_real_ptr' --type=rust -g '!**/target/**'
fd -t f 'class_field_inline_guard.rs' --exec rg -n -C 6 'sub\(I64|ptrtoint|select\(I1' {}

Repository: PerryTS/perry

Length of output: 151


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- candidate files ---'
fd -t f 'generic_dispatch.rs|class_field_inline_guard.rs' .

printf '%s\n' '--- relevant symbols and offsets ---'
rg -n -C 10 'safe_obj_handle|is_real_ptr|pic\.miss|sub\(I64|ptrtoint|select\(I1' \
  crates/perry-codegen/src/expr/property_get crates/perry-codegen \
  -g '*.rs' -g '!**/target/**' || true

printf '%s\n' '--- generic dispatch structure ---'
GENERIC=$(fd -t f 'generic_dispatch.rs' crates/perry-codegen | head -n 1)
if [ -n "${GENERIC}" ]; then
  wc -l "$GENERIC"
  ast-grep outline "$GENERIC"
fi

printf '%s\n' '--- class-field lowering structure ---'
CLASS=$(fd -t f 'class_field_inline_guard.rs' crates/perry-codegen | head -n 1)
if [ -n "${CLASS}" ]; then
  wc -l "$CLASS"
  ast-grep outline "$CLASS"
fi

Repository: PerryTS/perry

Length of output: 50372


🏁 Script executed:

#!/bin/bash
set -euo pipefail

GENERIC=$(fd -t f -a 'generic_dispatch.rs' crates/perry-codegen | head -n 1)
CLASS=$(fd -t f -a 'class_field_inline_guard.rs' crates/perry-codegen | head -n 1)

printf '%s\n' '--- generic dispatch sentinel and cache definitions ---'
rg -n -C 8 'cache_name|cache_ref|safe_obj_handle|is_real_ptr|PERRY_IC|pic\.miss' "$GENERIC"

printf '%s\n' '--- class-field guard implementation ---'
rg -n -C 12 'reserved|HAS_DESCRIPTORS|field_count|gc_type|sub\(I64|inttoptr|load' "$CLASS"

printf '%s\n' '--- cache global declarations and runtime layout references ---'
rg -n -C 8 'PIC_WAY|cache.*global|global.*cache|PERRY_IC|OBJ_FLAG_HAS_DESCRIPTORS|GcHeader|ObjectHeader' \
  crates/perry-codegen crates/perry-runtime crates -g '*.rs' -g '*.h' -g '*.c' \
  | head -n 1200

Repository: PerryTS/perry

Length of output: 50370


🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '%s\n' '--- inline-cache global emission ---'
rg -n -C 12 'ic_globals|perry_ic_|__ic_decl_|PIC_WAY_BASE|PIC_WAY_STATE' \
  crates/perry-codegen/src/module.rs crates/perry-codegen/src/function.rs \
  crates/perry-codegen/src -g '*.rs' | head -n 1000

printf '%s\n' '--- sanitizer and bounds-check configuration ---'
rg -n -C 8 'address sanitizer|address_sanitizer|asan|sanitize|bounds|UBSan|ubsan|sanitizer' \
  Cargo.toml .cargo crates scripts -g '*.toml' -g '*.rs' -g '*.py' -g '*.sh' 2>/dev/null | head -n 1000 || true

printf '%s\n' '--- cache layout declarations in all source files ---'
rg -n -C 6 'perry_ic_[0-9]|PERRY_IC_EPOCH|ic_globals' . \
  -g '!**/target/**' -g '!*.ll' -g '!*.o' -g '!*.bc' | head -n 1200

Repository: PerryTS/perry

Length of output: 50370


🏁 Script executed:

#!/bin/bash
set -euo pipefail

GENERIC=$(fd -t f -a 'generic_dispatch.rs' crates/perry-codegen | head -n 1)

printf '%s\n' '--- cache size and offsets ---'
rg -n -C 8 'PIC_CACHE_WORDS|PIC_WAY_BASE|PIC_WAY_STATE|cache_addr|safe_obj_handle' "$GENERIC"

printf '%s\n' '--- focused global emission ---'
sed -n '1048,1062p' crates/perry-codegen/src/codegen/method.rs
sed -n '1720,1735p' crates/perry-codegen/src/codegen/method.rs

printf '%s\n' '--- relevant IR safety tests ---'
rg -n -C 12 'safe_obj_handle|pic\.miss|small.handle|sentinel|negative|out.of.bounds|guard dominance|raw load' \
  crates/perry-codegen/src/expr/property_get/tests.rs \
  crates/perry-codegen/src/expr/property_get -g '*.rs' | head -n 1200

printf '%s\n' '--- available standalone instrumentation tools ---'
command -v clang || true
command -v opt || true
command -v llc || true
command -v llvm-as || true
command -v cargo || true

Repository: PerryTS/perry

Length of output: 50369


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
import re

generic = next(Path("crates/perry-codegen").rglob("generic_dispatch.rs"))
class_guard = next(Path("crates/perry-codegen").rglob("class_field_inline_guard.rs"))
text = generic.read_text()
class_text = class_guard.read_text()

words = int(re.search(r"PIC_CACHE_WORDS: usize = (\d+)", text).group(1))
global_bytes = words * 8

# Byte ranges read by the miss-block sentinel path, relative to safe_obj_handle.
reads = {
    "m_gc_type": (-8, 1),
    "m_magic": (12, 4),
    "m_ot": (0, 4),
    "m_res": (-6, 2),
    "m_keys": (16, 8),
    "m_pcid": (8, 4),
    "way_fc": (12, 4),
}
base = 0
outside = {
    name: (base + offset, base + offset + size)
    for name, (offset, size) in reads.items()
    if base + offset < 0 or base + offset + size > global_bytes
}

# A base eight bytes into the 12-word cache keeps every sentinel read in-range.
safe_base = 8
in_range_with_shift = all(
    0 <= safe_base + offset and safe_base + offset + size <= global_bytes
    for offset, size in reads.values()
)

print(f"PIC_CACHE_WORDS={words}, cache_bytes={global_bytes}")
print(f"out_of_range_at_cache_start={outside}")
print(f"all_reads_in_range_at_cache_plus_8={in_range_with_shift}")
print(f"class_guard_has_real_object_gate={all(x in class_text for x in (
    'above_band', 'ptr_safe', 'cond_br(&can_inline, &deref_label, &guardcall_label)'
))}")
assert outside == {"m_gc_type": (-8, -7), "m_res": (-6, -4)}
assert in_range_with_shift
PY

printf '%s\n' '--- tool availability ---'
for tool in clang opt llc llvm-as; do
  if command -v "$tool" >/dev/null 2>&1; then
    printf '%s: %s\n' "$tool" "$(command -v "$tool")"
  else
    printf '%s: unavailable\n' "$tool"
  fi
done

Repository: PerryTS/perry

Length of output: 379


🏁 Script executed:

#!/bin/bash
set -euo pipefail

python3 - <<'PY'
from pathlib import Path
import re

generic = next(Path("crates/perry-codegen").rglob("generic_dispatch.rs"))
class_guard = next(Path("crates/perry-codegen").rglob("class_field_inline_guard.rs"))
text = generic.read_text()
class_text = class_guard.read_text()

words = int(re.search(r"PIC_CACHE_WORDS: usize = (\d+)", text).group(1))
global_bytes = words * 8

reads = {
    "m_gc_type": (-8, 1),
    "m_magic": (12, 4),
    "m_ot": (0, 4),
    "m_res": (-6, 2),
    "m_keys": (16, 8),
    "m_pcid": (8, 4),
    "way_fc": (12, 4),
}
outside = {
    name: (offset, offset + size)
    for name, (offset, size) in reads.items()
    if offset < 0 or offset + size > global_bytes
}
safe_base = 8
in_range_with_shift = all(
    0 <= safe_base + offset
    and safe_base + offset + size <= global_bytes
    for offset, size in reads.values()
)
gate_tokens = (
    "above_band",
    "ptr_safe",
    "cond_br(&can_inline, &deref_label, &guardcall_label)",
)
has_gate = all(token in class_text for token in gate_tokens)

print(f"PIC_CACHE_WORDS={words}, cache_bytes={global_bytes}")
print(f"out_of_range_at_cache_start={outside}")
print(f"all_reads_in_range_at_cache_plus_8={in_range_with_shift}")
print(f"class_guard_has_real_object_gate={has_gate}")

assert outside == {"m_gc_type": (-8, -7), "m_res": (-6, -4)}
assert in_range_with_shift
assert has_gate
PY

printf '%s\n' '--- tool availability ---'
for tool in clang opt llc llvm-as; do
  if command -v "$tool" >/dev/null 2>&1; then
    printf '%s: %s\n' "$tool" "$(command -v "$tool")"
  else
    printf '%s: unavailable\n' "$tool"
  fi
done

Repository: PerryTS/perry

Length of output: 435


Use an in-bounds sentinel address for small handles.

When is_real_ptr is false, the miss block still executes the loads before is_object masks their results. @perry_ic_N is a 96-byte global, so offsets -8 and -6 access before its start and can trigger bounds or sanitizer failures. The class-field lowering is not equivalent because it performs these negative-offset loads only after proving a real heap pointer. Use an in-bounds sentinel, such as cache_addr + 8; all current reads then remain within the cache global.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@crates/perry-codegen/src/expr/property_get/generic_dispatch.rs` around lines
530 - 554, Use an in-bounds sentinel handle for non-real pointers by selecting
cache_addr + 8 instead of cache_addr in the safe_obj_handle computation. Keep
real pointers unchanged, and ensure the subsequent negative-offset loads in the
object checks remain within the `@perry_ic_N` cache global.

let m_keys_addr = ctx.block().add(I64, &safe_obj_handle, "16");
let m_keys_ptr = ctx.block().inttoptr(I64, &m_keys_addr);
let m_keys = ctx.block().load(I64, &m_keys_ptr);
let m_pcid_addr = ctx.block().add(I64, &safe_obj_handle, "8");
let m_pcid_ptr = ctx.block().inttoptr(I64, &m_pcid_addr);
let m_pcid = ctx.block().load(I32, &m_pcid_ptr);
let m_pcid_rel = ctx.block().add(I32, &m_pcid, "-2147483648");
let m_is_stamp = ctx.block().icmp_ult(I32, &m_pcid_rel, "1073741824");
let m_pcid64 = ctx.block().zext(I32, &m_pcid, I64);
let m_id_token = ctx.block().or(I64, &m_pcid64, "4611686018427387904");
let token = ctx
.block()
.select(I1, &m_is_stamp, I64, &m_id_token, &m_keys);
let token_nonnull = ctx.block().icmp_ne(I64, &token, "0");
let m_cache_epoch_ptr = ctx.block().gep(I64, &cache_ref, &[(I64, "2")]);
let m_cache_epoch = ctx.block().load(I64, &m_cache_epoch_ptr);
let m_live_epoch = ctx.block().load(I64, "@PERRY_IC_EPOCH");
let epoch_eq = ctx.block().icmp_eq(I64, &m_cache_epoch, &m_live_epoch);
crate::expr::emit_typed_feedback_record_call(
ctx.block(),
"js_typed_feedback_record_guard_fail",
Expand Down
Loading
Loading