Fix/gc malloc cse o3

tag.bat

@@ -1,2 +1,2 @@
-git tag -a v0.0-pre-alpha76 -m "pre alpha v0.0-76"
+git tag -a v0.0-pre-alpha77 -m "pre alpha v0.0-77"
 git push origin --tags

tag_del.bat

@@ -1,2 +1,2 @@
-git push --delete origin v0.0-pre-alpha76
-git tag -d v0.0-pre-alpha76
+git push --delete origin v0.0-pre-alpha77
+git tag -d v0.0-pre-alpha77

tslang/include/TypeScript/TypeScriptOps.td

@@ -1503,14 +1503,17 @@ def TypeScript_LogicalBinaryOp : TypeScript_Op<"LogicalBinary"> {
   let hasCanonicalizer = 1;
 }
 
-def TypeScript_CastOp : TypeScript_Op<"Cast", [DeclareOpInterfaceMethods<CastOpInterface>, Pure]> {
+def TypeScript_CastOp : TypeScript_Op<"Cast", [
+    DeclareOpInterfaceMethods<CastOpInterface>,
+    DeclareOpInterfaceMethods<MemoryEffectsOpInterface>
+]> {
   let description = [{
     Example:
       ts.cast %v : i32 to f16
   }];
   let arguments = (ins AnyType:$in);
   let results = (outs AnyType:$res);
-  
+
 
   let hasCanonicalizer = 1;
   let hasVerifier = 1;
@@ -1530,15 +1533,15 @@ def TypeScript_SafeCastOp : TypeScript_Op<"SafeCast", [
   let hasFolder = 1;
 }
 
-def TypeScript_BoxOp : TypeScript_Op<"Box", [Pure]> {
+def TypeScript_BoxOp : TypeScript_Op<"Box", [DeclareOpInterfaceMethods<MemoryEffectsOpInterface>]> {
   let description = [{
     Example:
       %ti = ts.typeof %1
       ts.box %v, %ti : i32 to ts.any
   }];
   let arguments = (ins AnyType:$in, TypeScript_String:$typeInfo);
   let results = (outs TypeScript_Any:$res);
-  
+
 }
 
 def TypeScript_UnboxOp : TypeScript_Op<"Unbox", [Pure]> {
@@ -1551,6 +1554,11 @@ def TypeScript_UnboxOp : TypeScript_Op<"Unbox", [Pure]> {
 
 }
 
+// Safe as Pure only because CreateUnionInstanceOpLowering calls CastLogicHelper::castLLVMTypes
+// (bitcast/struct-GEP only) directly, never CastLogicHelper::cast() - the dispatcher whose
+// castToArrayType branch is what made CastOp/BoxOp unsafely Pure (each called ch.MemoryAlloc
+// under the hood). If this op's lowering is ever changed to route through cast() instead,
+// re-audit whether it can still be Pure.
 def TypeScript_CreateUnionInstanceOp : TypeScript_Op<"CreateUnionInstance", [Pure]> {
   let description = [{
     Example:

tslang/lib/TypeScript/GCPass.cpp

@@ -153,6 +153,47 @@ class GCPass : public mlir::PassWrapper<GCPass, ModulePass>
         }
 
         funcOp->setAttr(SymbolTable::getSymbolAttrName(), mlir::StringAttr::get(funcOp->getContext(), newName));
+
+        markAsAllocatorIfNeeded(newName, funcOp);
+    }
+
+    // GC_malloc/GC_malloc_atomic/GC_memalign are fresh-pointer-per-call allocators, just like
+    // plain malloc. Unlike malloc, they aren't libc names LLVM's TargetLibraryInfo recognizes,
+    // so without explicit markings, GVN/EarlyCSE at -O3 see two calls with identical arguments
+    // (e.g. GC_malloc(0) for two different empty array fields) and no intervening memory
+    // clobber, and fold them into one shared allocation - aliasing fields that must stay
+    // distinct. memory_effects alone (writing unmodeled "other" memory) is not enough to stop
+    // GVN's call-CSE, which special-cases allocator-shaped functions via the `allockind` LLVM
+    // attribute (the same mechanism TargetLibraryInfo uses internally for malloc/calloc). Since
+    // GC_malloc isn't a recognized libc name, we must attach `allockind("alloc")` explicitly so
+    // each call is treated as returning a distinct, non-aliasing pointer.
+    void markAsAllocatorIfNeeded(StringRef newName, LLVM::LLVMFuncOp funcOp)
+    {
+        if (newName != "GC_malloc" && newName != "GC_malloc_atomic" && newName != "GC_memalign")
+        {
+            return;
+        }
+
+        auto *context = funcOp->getContext();
+        auto memoryEffects = LLVM::MemoryEffectsAttr::get(context, LLVM::ModRefInfo::Mod, LLVM::ModRefInfo::NoModRef,
+                                                            LLVM::ModRefInfo::NoModRef, LLVM::ModRefInfo::NoModRef,
+                                                            LLVM::ModRefInfo::NoModRef, LLVM::ModRefInfo::NoModRef);
+        funcOp.setMemoryEffectsAttr(memoryEffects);
+
+        // AllocFnKind::Alloc = 1<<0, Zeroed = 1<<4. GC_malloc/GC_malloc_atomic zero-fill;
+        // GC_memalign (GC_memalign) does not guarantee zeroing, so only mark Alloc for it.
+        uint64_t allocKind = newName == "GC_memalign" ? /*Alloc*/ 1 : /*Alloc|Zeroed*/ 1 | (1 << 4);
+        auto kindEntry = mlir::ArrayAttr::get(
+            context, {mlir::StringAttr::get(context, "allockind"),
+                      mlir::StringAttr::get(context, std::to_string(allocKind))});
+
+        llvm::SmallVector<mlir::Attribute> passthrough;
+        if (auto existing = funcOp.getPassthroughAttr())
+        {
+            passthrough.append(existing.begin(), existing.end());
+        }
+        passthrough.push_back(kindEntry);
+        funcOp.setPassthroughAttr(mlir::ArrayAttr::get(context, passthrough));
     }
 
     void renameCall(StringRef name, LLVM::CallOp callOp)
@@ -186,6 +227,7 @@ class GCPass : public mlir::PassWrapper<GCPass, ModulePass>
         LLVMCodeHelper ch(memSetCallOp, rewriter, nullptr, tsContext.compileOptions);
         auto i8PtrTy = th.getPtrType();
         auto gcInitFuncOp = ch.getOrInsertFunction("GC_malloc_atomic", th.getFunctionType(th.getPtrType(), mlir::ArrayRef<mlir::Type>{th.getI64Type()}));
+        markAsAllocatorIfNeeded("GC_malloc_atomic", gcInitFuncOp);
     }
 
     void injectInit(LLVM::LLVMFuncOp funcOp)

tslang/lib/TypeScript/LowerToLLVM.cpp

@@ -5515,6 +5515,29 @@ class GCMakeDescriptorOpLowering : public TsLlvmPattern<mlir_ts::GCMakeDescripto
     }
 };
 
+// GC_malloc_explicitly_typed is a fresh-pointer-per-call allocator, just like GC_malloc
+// (see GCPass.cpp markAsAllocatorIfNeeded for the full rationale). memory_effects alone
+// doesn't stop GVN/EarlyCSE at -O3 from merging two `new` sites with identical (size,
+// typeDescr) args - e.g. two instances of the same class - into one shared allocation;
+// the `allockind` LLVM attribute is what actually blocks that call-CSE.
+static void markGCMallocExplicitlyTypedAsAllocator(LLVM::LLVMFuncOp funcOp)
+{
+    auto *context = funcOp->getContext();
+
+    // AllocFnKind::Alloc = 1<<0; not marking Zeroed since GC_malloc_explicitly_typed's
+    // contents are then explicitly initialized by the caller.
+    auto kindEntry = mlir::ArrayAttr::get(
+        context, {mlir::StringAttr::get(context, "allockind"), mlir::StringAttr::get(context, "1")});
+
+    llvm::SmallVector<mlir::Attribute> passthrough;
+    if (auto existing = funcOp.getPassthroughAttr())
+    {
+        passthrough.append(existing.begin(), existing.end());
+    }
+    passthrough.push_back(kindEntry);
+    funcOp.setPassthroughAttr(mlir::ArrayAttr::get(context, passthrough));
+}
+
 class GCNewExplicitlyTypedOpLowering : public TsLlvmPattern<mlir_ts::GCNewExplicitlyTypedOp>
 {
   public:
@@ -5550,6 +5573,13 @@ class GCNewExplicitlyTypedOpLowering : public TsLlvmPattern<mlir_ts::GCNewExplic
         auto i8PtrTy = th.getPtrType();
 
         auto gcMallocExplicitlyTypedFunc = ch.getOrInsertFunction("GC_malloc_explicitly_typed", th.getFunctionType(i8PtrTy, {rewriter.getI64Type(), rewriter.getI64Type()}));
+        // Without this, two `new` sites with identical (size, typeDescr) args - e.g. two
+        // instances of the same class - look like redundant calls to GVN/EarlyCSE at -O3
+        // and get merged into one shared allocation. See GCPass.cpp markAsAllocatorIfNeeded.
+        gcMallocExplicitlyTypedFunc.setMemoryEffectsAttr(LLVM::MemoryEffectsAttr::get(
+            rewriter.getContext(), LLVM::ModRefInfo::Mod, LLVM::ModRefInfo::NoModRef, LLVM::ModRefInfo::NoModRef,
+            LLVM::ModRefInfo::NoModRef, LLVM::ModRefInfo::NoModRef, LLVM::ModRefInfo::NoModRef));
+        markGCMallocExplicitlyTypedAsAllocator(gcMallocExplicitlyTypedFunc);
         auto value = rewriter.create<LLVM::CallOp>(loc, gcMallocExplicitlyTypedFunc, ValueRange{sizeOfTypeValue, transformed.getTypeDescr()});
 
         rewriter.replaceOpWithNewOp<LLVM::BitcastOp>(op, resultType, value.getResult());

tslang/lib/TypeScript/TypeScriptOps.cpp

@@ -14,6 +14,7 @@
 #include "mlir/IR/PatternMatch.h"
 #include "mlir/IR/TypeUtilities.h"
 #include "mlir/Interfaces/FunctionImplementation.h"
+#include "mlir/Interfaces/SideEffectInterfaces.h"
 
 #include "llvm/ADT/TypeSwitch.h"
 #include "llvm/ADT/MapVector.h"
@@ -821,6 +822,40 @@ bool mlir_ts::CastOp::areCastCompatible(TypeRange inputs, TypeRange outputs)
     return true;
 }
 
+// Casting a ConstArrayType (e.g. the `[]` empty-array literal sentinel) to a heap-backed
+// ArrayType materializes a fresh GC allocation during lowering (see
+// CastLogicHelper::castToArrayType's `byValue` branch, which calls MemoryAlloc). Reporting no
+// effects here (as the previous blanket `Pure` trait did) let generic CSE - which runs on the
+// `ts` dialect before this op is ever lowered to the actual allocation call - treat two such
+// casts with the same (identical, CSE'd) constant operand as redundant and merge them into one,
+// silently aliasing what should be two distinct backing arrays. All other CastOp shapes are true
+// value-preserving casts with no allocation, so they keep reporting no effects.
+void mlir_ts::CastOp::getEffects(SmallVectorImpl<SideEffects::EffectInstance<MemoryEffects::Effect>> &effects)
+{
+    if (isa<mlir_ts::ConstArrayType>(getIn().getType()) && isa<mlir_ts::ArrayType>(getRes().getType()))
+    {
+        auto result = cast<OpResult>(getRes());
+        effects.emplace_back(MemoryEffects::Allocate::get(), result);
+        effects.emplace_back(MemoryEffects::Write::get(), result);
+    }
+}
+
+//===----------------------------------------------------------------------===//
+// BoxOp
+//===----------------------------------------------------------------------===//
+
+// BoxOp's lowering (AnyLogic::castToAny) unconditionally allocates a fresh "any" container via
+// MemoryAlloc on every execution - it is not a value-preserving reinterpretation like a real
+// cast. Reporting it as Pure (as before) would let CSE merge two structurally-identical Box ops
+// into one, aliasing what must be two distinct boxed values - the same bug fixed for CastOp
+// above. Always report the effect: unlike CastOp, every BoxOp shape allocates.
+void mlir_ts::BoxOp::getEffects(SmallVectorImpl<SideEffects::EffectInstance<MemoryEffects::Effect>> &effects)
+{
+    auto result = cast<OpResult>(getRes());
+    effects.emplace_back(MemoryEffects::Allocate::get(), result);
+    effects.emplace_back(MemoryEffects::Write::get(), result);
+}
+
 //===----------------------------------------------------------------------===//
 // DialectCastOp
 //===----------------------------------------------------------------------===//

tslang/test/tester/CMakeLists.txt

@@ -442,6 +442,7 @@ add_test(NAME test-compile-load-store-decorators COMMAND test-runner "${PROJECT_
 add_test(NAME test-compile-reference-null-bug COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00reference_null_bug.ts")
 add_test(NAME test-compile-reference-index-bug COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00reference_index_bug.ts")
 add_test(NAME test-compile-uint-compare-bug COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00uint_compare_bug.ts")
+add_test(NAME test-compile-gc-malloc-cse-o3 COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/gc_malloc_cse_o3.ts")
 
 add_test(NAME test-jit-00-print COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00print.ts")
 add_test(NAME test-jit-00-assert COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00assert.ts")
@@ -772,6 +773,7 @@ add_test(NAME test-jit-load-store-decorators COMMAND test-runner -jit "${PROJECT
 add_test(NAME test-jit-reference-null-bug COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00reference_null_bug.ts")
 add_test(NAME test-jit-reference-index-bug COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00reference_index_bug.ts")
 add_test(NAME test-jit-uint-compare-bug COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00uint_compare_bug.ts")
+add_test(NAME test-jit-gc-malloc-cse-o3 COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/gc_malloc_cse_o3.ts")
 
 # tesing include
 add_test(NAME test-compile-include-global-var COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/include_global_var.ts" "${PROJECT_SOURCE_DIR}/test/tester/tests/define_global_var.ts")

tslang/test/tester/tests/gc_malloc_cse_o3.ts

@@ -0,0 +1,49 @@
+// Regression test for: at -O3, two GC_malloc(0) calls with identical
+// arguments (allocating two distinct empty array fields in the same
+// constructor) were incorrectly CSE'd into a single shared allocation,
+// aliasing the two fields. Growing one field's backing array then
+// corrupted/orphaned the other field's pointer.
+//
+// Compile at --opt --opt_level=3 to exercise the bug; passes trivially
+// at --opt_level=0.
+
+class TwoArrays {
+    a: string[];
+    b: string[];
+
+    constructor() {
+        this.a = [];
+        this.b = [];
+    }
+
+    addA(v: string) {
+        this.a.push(v);
+    }
+
+    addB(v: string) {
+        this.b.push(v);
+    }
+}
+
+function main() {
+    const t = new TwoArrays();
+
+    // Grow `a` several times; if `a` and `b` alias the same backing
+    // store (the bug), this corrupts `b`'s view of its own array.
+    t.addA("a1");
+    t.addA("a2");
+    t.addA("a3");
+    t.addA("a4");
+
+    t.addB("b1");
+
+    assert(t.a.length == 4, "a.length should be 4");
+    assert(t.b.length == 1, "b.length should be 1 (aliasing bug would corrupt this)");
+    assert(t.a[0] == "a1");
+    assert(t.a[3] == "a4");
+    assert(t.b[0] == "b1");
+
+    print("done.");
+}
+
+main();