Skip to content

Commit 437cdd9

Browse files
authored
Revert "perf: bounds-check elimination in loop-indexed array access (#506)" (#511)
This reverts commit dcf1cd3. Co-authored-by: cs01 <cs01@users.noreply.github.com>
1 parent 76a1d9e commit 437cdd9

8 files changed

Lines changed: 6 additions & 557 deletions

File tree

src/codegen/expressions/access/index.ts

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@ export interface IndexAccessGeneratorContext {
3838
ensureDouble(value: string): string;
3939
setUsesJson(value: boolean): void;
4040
emitError(message: string, loc?: SourceLocation, suggestion?: string): never;
41-
isSafeIndex(indexName: string, arrayName: string): boolean;
4241
}
4342

4443
/**
@@ -182,19 +181,6 @@ export class IndexAccessGenerator {
182181
return indexValue;
183182
}
184183

185-
// Returns true when the given index access is a direct `arr[i]` pattern where
186-
// the (i, arr) pair has been proven safe by loop analysis, so we can skip the
187-
// runtime bounds check entirely.
188-
private isProvenSafeAccess(expr: IndexAccessNode | IndexAccessAssignmentNode): boolean {
189-
const obj = expr.object as ExprBase;
190-
const idx = expr.index as ExprBase;
191-
if (obj.type !== "variable") return false;
192-
if (idx.type !== "variable") return false;
193-
const arrName = (expr.object as VariableNode).name;
194-
const idxName = (expr.index as VariableNode).name;
195-
return this.ctx.isSafeIndex(idxName, arrName);
196-
}
197-
198184
private emitBoundsCheck(arrayPtr: string, arrayType: string, index: string): void {
199185
const lenPtr = this.ctx.nextTemp();
200186
this.ctx.emit(
@@ -232,9 +218,7 @@ export class IndexAccessGenerator {
232218
stringArrayPtr = cast;
233219
}
234220

235-
if (!this.isProvenSafeAccess(expr)) {
236-
this.emitBoundsCheck(stringArrayPtr, "%StringArray", index);
237-
}
221+
this.emitBoundsCheck(stringArrayPtr, "%StringArray", index);
238222

239223
const dataPtr = this.ctx.nextTemp();
240224
this.ctx.emit(
@@ -265,9 +249,7 @@ export class IndexAccessGenerator {
265249
arrayPtr = cast;
266250
}
267251

268-
if (!this.isProvenSafeAccess(expr)) {
269-
this.emitBoundsCheck(arrayPtr, "%Array", index);
270-
}
252+
this.emitBoundsCheck(arrayPtr, "%Array", index);
271253

272254
const dataPtr = this.ctx.nextTemp();
273255
this.ctx.emit(`${dataPtr} = getelementptr inbounds %Array, %Array* ${arrayPtr}, i32 0, i32 0`);
@@ -334,10 +316,9 @@ export class IndexAccessGenerator {
334316
const index = this.toI32Index(indexDouble);
335317
const contiguousStride = this.getContiguousStride(expr);
336318

337-
const safeAccess = this.isProvenSafeAccess(expr);
338319
const arrayType = this.ctx.getVariableType(arrayPtr);
339320
if (arrayType === "%ObjectArray*") {
340-
if (!safeAccess) this.emitBoundsCheck(arrayPtr, "%ObjectArray", index);
321+
this.emitBoundsCheck(arrayPtr, "%ObjectArray", index);
341322
if (contiguousStride > 0) {
342323
return this.emitContiguousElementPtr(arrayPtr, "%ObjectArray", index, contiguousStride);
343324
}
@@ -353,7 +334,7 @@ export class IndexAccessGenerator {
353334
if (arrayType === "i8*") {
354335
const arrayCast = this.ctx.nextTemp();
355336
this.ctx.emit(`${arrayCast} = bitcast i8* ${arrayPtr} to %ObjectArray*`);
356-
if (!safeAccess) this.emitBoundsCheck(arrayCast, "%ObjectArray", index);
337+
this.emitBoundsCheck(arrayCast, "%ObjectArray", index);
357338
if (contiguousStride > 0) {
358339
return this.emitContiguousElementPtr(arrayCast, "%ObjectArray", index, contiguousStride);
359340
}
@@ -366,7 +347,7 @@ export class IndexAccessGenerator {
366347
return this.emitPointerArrayElementPtr(data, index);
367348
}
368349

369-
if (!safeAccess) this.emitBoundsCheck(arrayPtr, "%ObjectArray", index);
350+
this.emitBoundsCheck(arrayPtr, "%ObjectArray", index);
370351
if (contiguousStride > 0) {
371352
return this.emitContiguousElementPtr(arrayPtr, "%ObjectArray", index, contiguousStride);
372353
}
@@ -385,9 +366,7 @@ export class IndexAccessGenerator {
385366

386367
const index = this.toI32Index(indexDouble);
387368

388-
if (!this.isProvenSafeAccess(expr)) {
389-
this.emitBoundsCheck(arrayPtr, "%Uint8Array", index);
390-
}
369+
this.emitBoundsCheck(arrayPtr, "%Uint8Array", index);
391370

392371
const dataFieldPtr = this.ctx.nextTemp();
393372
this.ctx.emit(

src/codegen/infrastructure/generator-context.ts

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -988,12 +988,6 @@ export interface IGeneratorContext {
988988
currentVarDeclKey: string | null;
989989
setCurrentVarDeclKey(key: string | null): void;
990990
getCurrentVarDeclKey(): string | null;
991-
992-
// Bounds-check elimination (perf) — IMPORTANT: keep at END of interface.
993-
pushSafeIndexScope(): void;
994-
popSafeIndexScope(): void;
995-
addSafeIndex(indexName: string, arrayName: string): void;
996-
isSafeIndex(indexName: string, arrayName: string): boolean;
997991
}
998992

999993
/**
@@ -2318,12 +2312,4 @@ export class MockGeneratorContext implements IGeneratorContext {
23182312
getCurrentVarDeclKey(): string | null {
23192313
return this.currentVarDeclKey;
23202314
}
2321-
2322-
// Bounds-check elimination stubs for MockGeneratorContext (no-ops).
2323-
pushSafeIndexScope(): void {}
2324-
popSafeIndexScope(): void {}
2325-
addSafeIndex(_indexName: string, _arrayName: string): void {}
2326-
isSafeIndex(_indexName: string, _arrayName: string): boolean {
2327-
return false;
2328-
}
23292315
}

src/codegen/llvm-generator.ts

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -330,14 +330,6 @@ export class LLVMGenerator extends BaseGenerator implements IGeneratorContext {
330330
public usesGC: number = 0;
331331
public usesMathRandom: number = 0;
332332

333-
// Bounds-check elimination: parallel arrays tracking (indexVar, arrayVar) pairs
334-
// that are proven safe within the currently active loop scope.
335-
// Parallel arrays used instead of Map for self-hosting compatibility.
336-
public safeIndexVars: string[] = [];
337-
public safeIndexArrays: string[] = [];
338-
// Per-loop-scope frame: each entry is how many pairs were added when that scope began.
339-
public safeIndexFrames: number[] = [];
340-
341333
public emitError(message: string, loc?: SourceLocation, suggestion?: string): never {
342334
this.diagnostics.error(message, loc, suggestion);
343335
const output = this.diagnostics.formatDiagnostic(
@@ -1685,39 +1677,6 @@ export class LLVMGenerator extends BaseGenerator implements IGeneratorContext {
16851677
// LLVMGenerator-specific fields not in BaseGenerator
16861678
this.stringBuilderSlen.clear();
16871679
this.stringBuilderScap.clear();
1688-
this.safeIndexVars.length = 0;
1689-
this.safeIndexArrays.length = 0;
1690-
this.safeIndexFrames.length = 0;
1691-
}
1692-
1693-
// Push a new loop scope for bounds-check elimination tracking
1694-
pushSafeIndexScope(): void {
1695-
this.safeIndexFrames.push(this.safeIndexVars.length);
1696-
}
1697-
1698-
// Pop the most recent loop scope, removing any pairs added within it
1699-
popSafeIndexScope(): void {
1700-
if (this.safeIndexFrames.length === 0) return;
1701-
const frameStart = this.safeIndexFrames[this.safeIndexFrames.length - 1];
1702-
this.safeIndexFrames.pop();
1703-
this.safeIndexVars.length = frameStart;
1704-
this.safeIndexArrays.length = frameStart;
1705-
}
1706-
1707-
// Record that indexName indexing into arrayName is proven in-bounds
1708-
addSafeIndex(indexName: string, arrayName: string): void {
1709-
this.safeIndexVars.push(indexName);
1710-
this.safeIndexArrays.push(arrayName);
1711-
}
1712-
1713-
// Check whether indexName indexing arrayName is known safe in some enclosing loop scope
1714-
isSafeIndex(indexName: string, arrayName: string): boolean {
1715-
for (let i = this.safeIndexVars.length - 1; i >= 0; i--) {
1716-
if (this.safeIndexVars[i] === indexName && this.safeIndexArrays[i] === arrayName) {
1717-
return true;
1718-
}
1719-
}
1720-
return false;
17211680
}
17221681

17231682
getThisPointer(): string | null {

src/codegen/statements/control-flow.ts

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,6 @@ import type { FieldInfo } from "../infrastructure/type-resolver/types.js";
3232
import { stripOptional } from "../infrastructure/type-system.js";
3333
import { setWantsI1 } from "../expressions/condition-generator.js";
3434
import { tryOptimizeWhileLoopMap } from "./loop-idiom.js";
35-
import { analyzeWhileSafety, analyzeForSafety } from "./loop-safety.js";
3635

3736
interface ExprBase {
3837
type: string;
@@ -324,17 +323,7 @@ export class ControlFlowGenerator {
324323
this.ctx.setCurrentLabel(bodyLabel);
325324
this.loopContinueLabels.push(condLabel);
326325
this.loopBreakLabels.push(endLabel);
327-
328-
// Bounds-check elimination: if the loop matches a safe pattern, register
329-
// the (indexVar, arrayVar) pair so arr[i] inside body skips its bounds check.
330-
const whileSafePat = analyzeWhileSafety(whileStmt);
331-
this.ctx.pushSafeIndexScope();
332-
if (whileSafePat) {
333-
this.ctx.addSafeIndex(whileSafePat.indexName, whileSafePat.arrayName);
334-
}
335-
336326
this.ctx.generateBlock(whileStmt.body, params);
337-
this.ctx.popSafeIndexScope();
338327
this.loopContinueLabels.pop();
339328
this.loopBreakLabels.pop();
340329
const bodyHasTerminator = this.ctx.lastInstructionIsTerminator();
@@ -486,16 +475,7 @@ export class ControlFlowGenerator {
486475
this.ctx.setCurrentLabel(bodyLabel);
487476
this.loopContinueLabels.push(updateLabel);
488477
this.loopBreakLabels.push(endLabel);
489-
490-
// Bounds-check elimination: classic-for-loop safe index registration.
491-
const forSafePat = analyzeForSafety(forStmt as ForStatement);
492-
this.ctx.pushSafeIndexScope();
493-
if (forSafePat) {
494-
this.ctx.addSafeIndex(forSafePat.indexName, forSafePat.arrayName);
495-
}
496-
497478
this.ctx.generateBlock(forStmt.body, params);
498-
this.ctx.popSafeIndexScope();
499479
this.loopContinueLabels.pop();
500480
this.loopBreakLabels.pop();
501481
const bodyHasTerminator3 = this.ctx.lastInstructionIsTerminator();

0 commit comments

Comments
 (0)