A global initialised from another global runs after it (#446)

tslang/lib/TypeScript/LowerToLLVM.cpp

@@ -61,6 +61,13 @@ struct TsLlvmContext
     TsLlvmContext(CompileOptions& compileOptions) : compileOptions(compileOptions) {};
 
     CompileOptions& compileOptions;
+
+    // the priority of the next global's own constructor, above every one MLIRGen made: LLVM leaves
+    // the order of equal priorities undefined, and a global may read one made before it (#446)
+    int64_t nextGlobalConstructorPriority = LAST_GLOBAL_CONSTRUCTOR_PRIORITY;
+
+    // the calls put into __mlir_gctors so far, with their priorities, to keep them in that order
+    std::vector<std::pair<int64_t, mlir::Operation *>> globalConstructorCalls;
 };
 
 template <typename OpTy> class TsLlvmPattern : public OpConversionPattern<OpTy>
@@ -4511,7 +4518,8 @@ struct GlobalOpLowering : public TsLlvmPattern<mlir_ts::GlobalOp>
                 lch.seekLastOp<mlir_ts::GlobalConstructorOp>(parentModule.getBody());
 
                 rewriter.create<mlir_ts::GlobalConstructorOp>(loc, 
-                    FlatSymbolRefAttr::get(rewriter.getContext(), StringRef(name)), rewriter.getIndexAttr(1000));
+                    FlatSymbolRefAttr::get(rewriter.getContext(), StringRef(name)),
+                    rewriter.getIndexAttr(tsLlvmContext->nextGlobalConstructorPriority++));
             }
         }
         else
@@ -6725,6 +6733,27 @@ class SwitchStateInternalOpLowering : public TsLlvmPattern<mlir_ts::SwitchStateI
     }
 };
 
+// the body of __mlir_gctors, whichever function op it is by now
+static mlir::Region *gctorsBodyOf(mlir::Operation *op)
+{
+    if (auto llvmFunc = dyn_cast_or_null<LLVM::LLVMFuncOp>(op))
+    {
+        return &llvmFunc.getBody();
+    }
+
+    if (auto tsFunc = dyn_cast_or_null<mlir_ts::FuncOp>(op))
+    {
+        return &tsFunc.getBody();
+    }
+
+    if (auto func = dyn_cast_or_null<func::FuncOp>(op))
+    {
+        return &func.getBody();
+    }
+
+    return nullptr;
+}
+
 struct GlobalConstructorOpLowering : public TsLlvmPattern<mlir_ts::GlobalConstructorOp>
 {
     using TsLlvmPattern<mlir_ts::GlobalConstructorOp>::TsLlvmPattern;
@@ -6762,51 +6791,37 @@ struct GlobalConstructorOpLowering : public TsLlvmPattern<mlir_ts::GlobalConstru
                 auto &entryBlock = *initFunc.addEntryBlock();
                 rewriter.setInsertionPointToEnd(&entryBlock);
 
-                rewriter.create<LLVM::CallOp>(loc, TypeRange{}, globalConstructorOp.getGlobalNameAttr(), ValueRange{});
+                auto callOp = rewriter.create<LLVM::CallOp>(loc, TypeRange{}, globalConstructorOp.getGlobalNameAttr(), ValueRange{});
+                tsLlvmContext->globalConstructorCalls.push_back({(int64_t)priority, callOp});
                 rewriter.create<LLVM::ReturnOp>(loc, ValueRange{});
             }
-            else if (auto llvmGCtors = dyn_cast_or_null<LLVM::LLVMFuncOp>(mlirGCtors))
+            else if (auto *gctorsBody = gctorsBodyOf(mlirGCtors))
             {
+                // in priority order, each after the ones before it (#446): before the first call
+                // already here with a higher priority, else after the last one here
                 OpBuilder::InsertionGuard insertGuard(rewriter);
-                if (priority <= 1000)
+                auto &calls = tsLlvmContext->globalConstructorCalls;
+                auto later = llvm::find_if(calls, [&](auto &entry) { return entry.first > (int64_t)priority; });
+                if (later != calls.end())
                 {
-                    rewriter.setInsertionPointToStart(&llvmGCtors.getBody().front());
+                    rewriter.setInsertionPoint(later->second);
                 }
-                else
+                else if (!calls.empty())
                 {
-                    rewriter.setInsertionPoint(llvmGCtors.getBody().back().getTerminator());
+                    rewriter.setInsertionPointAfter(calls.back().second);
                 }
-
-                rewriter.create<LLVM::CallOp>(loc, TypeRange{}, globalConstructorOp.getGlobalNameAttr(), ValueRange{});
-            }
-            else if (auto tsFuncGCtors = dyn_cast_or_null<mlir_ts::FuncOp>(mlirGCtors))
-            {
-                OpBuilder::InsertionGuard insertGuard(rewriter);
-                if (priority <= 1000)
+                else if (priority <= 1000)
                 {
-                    rewriter.setInsertionPointToStart(&tsFuncGCtors.getBody().front());
+                    rewriter.setInsertionPointToStart(&gctorsBody->front());
                 }
                 else
                 {
-                    rewriter.setInsertionPoint(tsFuncGCtors.getBody().back().getTerminator());
+                    rewriter.setInsertionPoint(gctorsBody->back().getTerminator());
                 }
 
-                rewriter.create<LLVM::CallOp>(loc, TypeRange{}, globalConstructorOp.getGlobalNameAttr(), ValueRange{});
+                auto callOp = rewriter.create<LLVM::CallOp>(loc, TypeRange{}, globalConstructorOp.getGlobalNameAttr(), ValueRange{});
+                calls.insert(later, {(int64_t)priority, callOp});
             }
-            else if (auto funcGCtors = dyn_cast_or_null<func::FuncOp>(mlirGCtors))
-            {
-                OpBuilder::InsertionGuard insertGuard(rewriter);
-                if (priority <= 1000)
-                {
-                    rewriter.setInsertionPointToStart(&funcGCtors.getBody().front());
-                }
-                else
-                {
-                    rewriter.setInsertionPoint(funcGCtors.getBody().back().getTerminator());
-                }
-
-                rewriter.create<LLVM::CallOp>(loc, TypeRange{}, globalConstructorOp.getGlobalNameAttr(), ValueRange{});
-            }            
             else 
             {
                 assert(false);
@@ -8127,6 +8142,12 @@ void TypeScriptToLLVMLoweringPass::runOnOperation()
 
     // The only remaining operation to lower from the `typescript` dialect, is the PrintOp.
     TsLlvmContext tsLlvmContext{tsContext.compileOptions};
+    for (auto existing : m.getBody()->getOps<mlir_ts::GlobalConstructorOp>())
+    {
+        tsLlvmContext.nextGlobalConstructorPriority =
+            std::max<int64_t>(tsLlvmContext.nextGlobalConstructorPriority, existing.getPriority().getSExtValue() + 1);
+    }
+
     patterns.insert<
         AddressOfOpLowering, ArithmeticUnaryOpLowering, ArithmeticBinaryOpLowering,
         AssertOpLowering, CastOpLowering, ConstantOpLowering, DefaultOpLowering, 

tslang/lib/TypeScript/MLIRGenModule.cpp

@@ -728,9 +728,16 @@ namespace mlirgen
         builder.setInsertionPointToStart(theModule.getBody());
         mclh.seekLastOp<mlir_ts::GlobalConstructorOp>(theModule.getBody());
 
+        // each one after every one before it: LLVM leaves the order of equal priorities undefined,
+        // and a global's initializer may read one made before it (#446)
+        int64_t priority = LAST_GLOBAL_CONSTRUCTOR_PRIORITY;
+        for (auto existing : theModule.getBody()->getOps<mlir_ts::GlobalConstructorOp>())
+        {
+            priority = std::max<int64_t>(priority, existing.getPriority().getSExtValue() + 1);
+        }
+
         builder.create<mlir_ts::GlobalConstructorOp>(
-            location, mlir::FlatSymbolRefAttr::get(builder.getContext(), funcName),
-            builder.getIndexAttr(LAST_GLOBAL_CONSTRUCTOR_PRIORITY));
+            location, mlir::FlatSymbolRefAttr::get(builder.getContext(), funcName), builder.getIndexAttr(priority));
     }
 
     mlir::LogicalResult MLIRGenImpl::generateGlobalEntryCode(mlir::Location location, NodeArray<Statement> statements,

tslang/test/tester/CMakeLists.txt

@@ -478,6 +478,7 @@ tslang_add_test(NAME test-compile-00-nullable-to-any COMMAND test-runner "${PROJ
 tslang_add_test(NAME test-compile-00-ternary-new-or-null COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00ternary_new_or_null.ts")
 tslang_add_test(NAME test-compile-00-class COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts")
 tslang_add_test(NAME test-compile-00-class-new COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_new.ts")
+tslang_add_test(NAME test-compile-00-global-from-global COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_from_global.ts")
 tslang_add_test(NAME test-compile-00-try-cleanup-then-catch COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00try_cleanup_then_catch.ts")
 tslang_add_test(NAME test-compile-00-global-union-from-call COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_union_from_call.ts")
 tslang_add_test(NAME test-compile-00-capture-in-new-arguments COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00capture_in_new_arguments.ts")
@@ -970,6 +971,7 @@ tslang_add_test(NAME test-jit-00-nullable-to-any COMMAND test-runner -jit "${PRO
 tslang_add_test(NAME test-jit-00-ternary-new-or-null COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00ternary_new_or_null.ts")
 tslang_add_test(NAME test-jit-00-class COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class.ts")
 tslang_add_test(NAME test-jit-00-class-new COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00class_new.ts")
+tslang_add_test(NAME test-jit-00-global-from-global COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_from_global.ts")
 tslang_add_test(NAME test-jit-00-try-cleanup-then-catch COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00try_cleanup_then_catch.ts")
 tslang_add_test(NAME test-jit-00-global-union-from-call COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00global_union_from_call.ts")
 tslang_add_test(NAME test-jit-00-capture-in-new-arguments COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00capture_in_new_arguments.ts")
@@ -1781,6 +1783,7 @@ set(TSLANG_CORPUS
     00if_return.ts
     00in_method_names.ts
     00in.ts
+    00global_from_global.ts
     00try_cleanup_then_catch.ts
     00global_object_literal.ts
     00infer.ts

tslang/test/tester/tests/00global_from_global.ts

@@ -0,0 +1,33 @@
+// A module-level variable initialised from another one reads that one's value (#446). Each global
+// that runs code gets a constructor, all at the same priority, and LLVM leaves the order of equal
+// priorities undefined: optimised, `b`'s constructor was folded before `a`'s and read garbage; the
+// JIT ran them in reverse.
+class C {
+    v = 5;
+}
+
+function mk(v: number) {
+    const c = new C();
+    c.v = v;
+    return c;
+}
+
+let a: number[] = [1, 2];
+let b = a;
+const c = new C();
+const d = c;
+const r = mk(7);
+let e: C | null = r;
+let s = "text" + 1;
+let t = s;
+let n = a.length + 1;
+
+function main() {
+    assert(b.length == 2 && b[1] == 2, "an array global from another");
+    assert(d.v == 5, "a class global from another");
+    assert(e !== null && e.v == 7, "a union global from another");
+    assert(t == "text1", "a string global from another");
+    assert(n == 3, "a number computed from another global");
+
+    print("done.");
+}