Skip to content

Commit 26337ce

Browse files
Split MLIRGen.cpp: extract MLIRGenImpl into private header, move cast family to MLIRGenCast.cpp (review doc 1)
Mechanical first slice of the TU split: MLIRGenImpl moves from MLIRGen.cpp's anonymous namespace into typescript::mlirgen in the private header MLIRGenImpl.h (bodies still inline; later slices move them out). MLIRGenCast.cpp is the first satellite TU with the 29 cast-family methods defined out-of-line. Enabling fixes: GenContext/PassResult/ValueOrLogicalResult had internal linkage (anonymous namespace in MLIRGenContext.h) and now live in namespace typescript; ts::print(Node) in dump.h is now inline; compileOptionsPtr stays TU-local behind setCompileOptions(). Verified: MLIRGenTests 12/12, ctest JIT 339/339, AOT 342/342. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent aaef434 commit 26337ce

7 files changed

Lines changed: 26839 additions & 26733 deletions

File tree

docs/MLIRGen-refactoring-review.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ Benefits: parallel builds (this single TU dominates incremental build time), sma
3939

4040
Mechanically safest path: keep `MLIRGenImpl` as-is, move method *bodies* out with no signature changes, verify with the existing test suite after each batch.
4141

42+
*Status: in progress.* First slice landed: the class moved out of MLIRGen.cpp's anonymous namespace into `typescript::mlirgen` in a private header `lib/TypeScript/MLIRGenImpl.h` (bodies still inline; each subsequent slice replaces a family's inline bodies with declarations and moves the definitions to a new TU). `MLIRGenCast.cpp` is the first satellite: the contiguous cast family (`selectFieldsValues` … `createBoundMethodFromExtensionMethod`, 29 methods) defined out-of-line. Enabling fixes: `GenContext`/`PassResult`/`ValueOrLogicalResult` in `MLIRGenContext.h` were in an anonymous namespace **in a header** (each TU got distinct types — worked only because there was one TU); they now live in `namespace typescript`. `ts::print(Node)` in `dump.h` was a non-inline header definition — now `inline`. The TU-static `compileOptionsPtr` stays in MLIRGen.cpp behind a new `setCompileOptions()` so inline code never references a TU-local. Remaining units to peel off per the table above; header hygiene (§8) comes after the last one.
43+
4244
## 2. Kill the dangling-`FuncOp` hazard in the symbol maps
4345

4446
`functionMap` (per-namespace `llvm::StringMap<mlir_ts::FuncOp>`) and `GenericFunctionInfo::funcOp` cache **raw op handles**. Discovery passes create ops and then erase them (`clearTempModule`, and until recently `theModule.getBody()->clear()`), so any cached handle from a discarded pass dangles. Today the code survives because consumers only read the *type* early — but this is exactly what made the nested-import bug subtle.

tslang/include/TypeScript/MLIRLogic/MLIRGenContext.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,9 @@ using llvm::SmallVector;
3939
using llvm::StringRef;
4040
using llvm::Twine;
4141

42-
namespace
42+
// These types are shared across the MLIRGen translation units (see MLIRGenImpl.h),
43+
// so they need external linkage — an anonymous namespace would give each TU its own type.
44+
namespace typescript
4345
{
4446

4547
struct PassResult
@@ -213,6 +215,6 @@ struct ValueOrLogicalResult
213215
}
214216
};
215217

216-
} // namespace
218+
} // namespace typescript
217219

218220
#endif // MLIR_TYPESCRIPT_MLIRGENCONTEXT_H_

tslang/lib/TypeScript/CMakeLists.txt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ add_mlir_dialect_library(MLIRTypeScript
1717
AsyncDialectTranslation.cpp
1818
DiagnosticHelper.cpp
1919
MLIRGen.cpp
20-
LowerToAffineLoops.cpp
20+
MLIRGenCast.cpp
21+
LowerToAffineLoops.cpp
2122
LowerToLLVM.cpp
2223
RelocateConstantPass.cpp
2324
GCPass.cpp

0 commit comments

Comments
 (0)