diff --git a/tslang/lib/TypeScript/LowerToLLVM.cpp b/tslang/lib/TypeScript/LowerToLLVM.cpp index 30a50493a..e0f2ae665 100644 --- a/tslang/lib/TypeScript/LowerToLLVM.cpp +++ b/tslang/lib/TypeScript/LowerToLLVM.cpp @@ -2795,7 +2795,9 @@ struct NewArrayOpLowering : public TsLlvmPattern auto multSizeOfTypeValue = rewriter.create(loc, llvmIndexType, ValueRange{sizeOfTypeValue, countAsIndexType}); - auto allocated = ch.MemoryAlloc(multSizeOfTypeValue); + // zeroed: its elements are not set yet, and under `-mm=rc` the first store into one + // releases what the slot held - `malloc`'s leftovers would be released as a reference + auto allocated = ch.MemoryAlloc(multSizeOfTypeValue, MemoryAllocSet::Zero); // create array type auto llvmRtArrayStructType = tch.convertType(arrayType); diff --git a/tslang/lib/TypeScript/MLIRGenCast.cpp b/tslang/lib/TypeScript/MLIRGenCast.cpp index bcecba2ec..ae26d491d 100644 --- a/tslang/lib/TypeScript/MLIRGenCast.cpp +++ b/tslang/lib/TypeScript/MLIRGenCast.cpp @@ -928,64 +928,25 @@ namespace mlirgen } auto constArrayType = dyn_cast(valueType); - - // A `const` array literal (`const c = [1, 2]`) now gets real identity - // storage so mutating methods like .sort() share one heap array instead of - // a fresh disposable copy per call (see processConstRef / - // adjustLocalVariableType / adjustGlobalVariableType in MLIRGenImpl.h) -- - // which means reading `c` elsewhere yields the already-widened - // `!ts.array`, not the original `!ts.const_array` this function - // keys off. Trace back through whichever shape that widening introduced to - // recover the original literal, so the elementwise conversion below still - // applies to a `const` int-array literal read into e.g. `number[]`: - // - local: Load -> VariableOp -> its initializer (a Cast chain) - // - module-level: Load -> AddressOf(@global) -> that GlobalOp's - // initializer region, whose GlobalResultOp terminator yields the same - // kind of Cast chain (see createGlobalVariableInitialization) - mlir::Value literalValue = value; if (!constArrayType) { - if (auto loadOp = value.getDefiningOp()) - { - if (auto varOp = loadOp.getReference().getDefiningOp()) - { - if (auto init = varOp.getInitializer()) - { - literalValue = init; - } - } - else if (auto addressOfOp = loadOp.getReference().getDefiningOp()) - { - if (auto globalOp = theModule.lookupSymbol(addressOfOp.getGlobalNameAttr())) - { - if (auto *initBlock = globalOp.getInitializerBlock()) - { - if (auto globalResultOp = dyn_cast(initBlock->getTerminator())) - { - if (globalResultOp.getResults().size() == 1) - { - literalValue = globalResultOp.getResults().front(); - } - } - } - } - } - } - - while (auto castOp = literalValue.getDefiningOp()) + // a `const c = [1, 2]` read back is already the array its storage widened it to (see + // processConstRef / adjustLocalVariableType / adjustGlobalVariableType in MLIRGenImpl.h), + // and may have been changed since: its elements are converted as they are now + if (auto srcArrayType = dyn_cast(valueType)) { - literalValue = castOp.getIn(); + return castArrayElementwise(location, arrayType, value, srcArrayType, genContext); } - constArrayType = dyn_cast(literalValue.getType()); + return std::nullopt; } - if (!constArrayType || constArrayType.getElementType() == arrayType.getElementType()) + if (constArrayType.getElementType() == arrayType.getElementType()) { return std::nullopt; } - auto constOp = literalValue.getDefiningOp(); + auto constOp = value.getDefiningOp(); auto elementAttrs = constOp ? dyn_cast(constOp.getValue()) : mlir::ArrayAttr(); if (!elementAttrs) { @@ -1035,6 +996,87 @@ namespace mlirgen return V(builder.create(location, arrayType, elements)); } + // An array of numbers held as `s32` (or another integer or float type) keeps that layout, so it is + // not shared as a `number[]`: a new array is built, each element cast to the element type as it is + // assigned - nested arrays in turn. The new array is a copy, so later changes to one of them are not + // seen through the other. An array literal met at a union or a `const` binding gets here with s32 + // elements (see castConstArrayToArray), and so does a variable holding one. + std::optional MLIRGenImpl::castArrayElementwise(mlir::Location location, mlir_ts::ArrayType arrayType, mlir::Value value, mlir_ts::ArrayType srcArrayType, const GenContext &genContext) + { + auto isNumeric = [](mlir::Type type) { + return isa(type) || isa(type) || isa(type); + }; + + std::function isNumericSource = [&](mlir::Type type) { + if (auto nestedArrayType = dyn_cast(type)) + { + return isNumericSource(nestedArrayType.getElementType()); + } + + return isa(type) || isa(type); + }; + + // what a number can be assigned to: another number type, `any`, or a union holding a number + std::function isNumericTarget = [&](mlir::Type type) { + if (auto nestedArrayType = dyn_cast(type)) + { + return isNumericTarget(nestedArrayType.getElementType()); + } + + if (auto unionType = dyn_cast(type)) + { + return llvm::any_of(unionType.getTypes(), isNumeric); + } + + return isNumeric(type) || isa(type) || isa(type); + }; + + auto srcElementType = srcArrayType.getElementType(); + auto elementType = arrayType.getElementType(); + if (srcElementType == elementType || !isNumericSource(srcElementType) || !isNumericTarget(elementType)) + { + return std::nullopt; + } + + SymbolTableScopeT varScope(symbolTable); + + auto length = builder.create(location, builder.getIndexType(), value); + auto newArray = builder.create(location, arrayType, length); + + auto srcArrayVarDecl = std::make_shared(".src_array", srcArrayType, location); + DECLARE(srcArrayVarDecl, value); + + auto dstArrayVarDecl = std::make_shared(".dst_array", arrayType, location); + DECLARE(dstArrayVarDecl, newArray); + + NodeFactory nf(NodeFactoryFlags::None); + + auto _src_array_ident = nf.createIdentifier(S(".src_array")); + auto _dst_array_ident = nf.createIdentifier(S(".dst_array")); + auto _i = nf.createIdentifier(S(".i")); + + // for (let .i = 0; .i < .src_array.length; ++.i) .dst_array[.i] = .src_array[.i]; + NodeArray declarations; + declarations.push_back(nf.createVariableDeclaration(_i, undefined, undefined, nf.createNumericLiteral(S("0")))); + auto initVars = nf.createVariableDeclarationList(declarations, NodeFlags::Let); + + auto cond = nf.createBinaryExpression(_i, nf.createToken(SyntaxKind::LessThanToken), + nf.createPropertyAccessExpression(_src_array_ident, nf.createIdentifier(S(LENGTH_FIELD_NAME)))); + + auto incr = nf.createPrefixUnaryExpression(nf.createToken(SyntaxKind::PlusPlusToken), _i); + + auto assign = nf.createExpressionStatement( + nf.createBinaryExpression(nf.createElementAccessExpression(_dst_array_ident, _i), nf.createToken(SyntaxKind::EqualsToken), + nf.createElementAccessExpression(_src_array_ident, _i))); + + if (mlir::failed(mlirGen(nf.createForStatement(initVars, cond, incr, assign), genContext))) + { + return mlir::failure(); + } + + return V(newArray); + } + std::optional MLIRGenImpl::castTupleLikeVariants(mlir::Location location, mlir::Type type, mlir::Value value, mlir::Type valueType, const GenContext &genContext) { // const tuple to tuple diff --git a/tslang/lib/TypeScript/MLIRGenImpl.h b/tslang/lib/TypeScript/MLIRGenImpl.h index 112f6124a..33f572d0d 100644 --- a/tslang/lib/TypeScript/MLIRGenImpl.h +++ b/tslang/lib/TypeScript/MLIRGenImpl.h @@ -11121,6 +11121,7 @@ class MLIRGenImpl // constant array literal to an array of another element type, element by element std::optional castConstArrayToArray(mlir::Location location, mlir::Type type, mlir::Value value, mlir::Type valueType, const GenContext &genContext); + std::optional castArrayElementwise(mlir::Location location, mlir_ts::ArrayType arrayType, mlir::Value value, mlir_ts::ArrayType srcArrayType, const GenContext &genContext); // reports which part of a function signature does not match the one it is assigned to void emitFunctionTypeMismatch(mlir::Location location, mlir::Type valueType, mlir::Type type, MatchResult match); diff --git a/tslang/test/tester/CMakeLists.txt b/tslang/test/tester/CMakeLists.txt index e5d4088fc..6c427dae8 100644 --- a/tslang/test/tester/CMakeLists.txt +++ b/tslang/test/tester/CMakeLists.txt @@ -296,6 +296,7 @@ tslang_add_test(NAME test-compile-00-typeof-function-narrowing COMMAND test-runn tslang_add_test(NAME test-compile-00-typeof-static-fold-conditions COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00typeof_static_fold_conditions.ts") tslang_add_test(NAME test-compile-00-typeof-union-narrowing COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00typeof_union_narrowing.ts") tslang_add_test(NAME test-compile-00-const-array-to-array-elements COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_array_to_array_elements.ts") +tslang_add_test(NAME test-compile-00-array-widen-elements-now COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_widen_elements_now.ts") tslang_add_test(NAME test-compile-00-const-array-nested-elements COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_array_nested_elements.ts") tslang_add_test(NAME test-compile-00-const-array-mutation COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_array_mutation.ts") tslang_add_test(NAME test-compile-00-array-move-wide-elements COMMAND test-runner "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_move_wide_elements.ts") @@ -731,6 +732,7 @@ tslang_add_test(NAME test-jit-00-typeof-function-narrowing COMMAND test-runner - tslang_add_test(NAME test-jit-00-typeof-static-fold-conditions COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00typeof_static_fold_conditions.ts") tslang_add_test(NAME test-jit-00-typeof-union-narrowing COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00typeof_union_narrowing.ts") tslang_add_test(NAME test-jit-00-const-array-to-array-elements COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_array_to_array_elements.ts") +tslang_add_test(NAME test-jit-00-array-widen-elements-now COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_widen_elements_now.ts") tslang_add_test(NAME test-jit-00-const-array-nested-elements COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_array_nested_elements.ts") tslang_add_test(NAME test-jit-00-const-array-mutation COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00const_array_mutation.ts") tslang_add_test(NAME test-jit-00-array-move-wide-elements COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00array_move_wide_elements.ts") @@ -1477,6 +1479,7 @@ set(TSLANG_CORPUS 00conditional_owned_result.ts 00const_array_nested_elements.ts 00const_array_to_array_elements.ts + 00array_widen_elements_now.ts 00owned_array_splice.ts 00alloc_in_catch.ts 00any_compare.ts diff --git a/tslang/test/tester/tests/00array_widen_elements_now.ts b/tslang/test/tester/tests/00array_widen_elements_now.ts new file mode 100644 index 000000000..42e2bcea9 --- /dev/null +++ b/tslang/test/tester/tests/00array_widen_elements_now.ts @@ -0,0 +1,73 @@ +// An integer array literal keeps its `s32` elements, and an array held in a variable is converted +// element by element when it is read as `number[]`. The conversion used to trace the variable back +// to the literal it was declared with and build the array from that literal, so everything done to +// the array since - push, element writes, a new array assigned, a sort - was silently lost. +const moduleArray = [7, 8, 9]; +moduleArray[0] = 70; +moduleArray.push(10); +const moduleWidened: number[] = moduleArray; + +function widen(a: s32[]): number[] { + return a; +} + +function main() { + let pushed = [1, 2]; + pushed.push(3); + pushed[0] = 9; + const fromLet: number[] = pushed; + assert(fromLet.length == 3, "let: pushed element counted"); + assert(fromLet[0] == 9, "let: written element"); + assert(fromLet[2] / 2 == 1.5, "let: pushed element, number arithmetic"); + + const written = [3, 1, 2]; + written[0] = 42; + const fromConst: number[] = written; + assert(fromConst[0] == 42, "const: written element"); + assert(fromConst[1] == 1, "const: untouched element"); + + let reassigned = [1, 2]; + reassigned = [5, 6, 7]; + const fromReassigned: number[] = reassigned; + assert(fromReassigned.length == 3, "reassigned: length"); + assert(fromReassigned[0] == 5, "reassigned: element"); + + // no literal to trace back to at all: a parameter + const fromParameter = widen(pushed); + assert(fromParameter.length == 3, "parameter: length"); + assert(fromParameter[0] / 2 == 4.5, "parameter: element, number arithmetic"); + + const nested = [[1, 2], [3]]; + const fromNested: number[][] = nested; + assert(fromNested.length == 2, "nested: outer length"); + assert(fromNested[1].length == 1, "nested: inner length"); + assert(fromNested[0][1] / 4 == 0.5, "nested: element, number arithmetic"); + + const toUnion: (number | string)[] = written; + assert(toUnion[0] == 42, "union element type"); + + const toAny: any[] = written; + assert(toAny.length == 3, "any element type: length"); + + const empty: s32[] = []; + const fromEmpty: number[] = empty; + assert(fromEmpty.length == 0, "empty array"); + + // the widened array is a copy: a write to it does not reach the original + const copy: number[] = written; + copy[1] = 100; + assert(written[1] == 1, "copy: original unchanged"); + assert(copy[1] == 100, "copy: written"); + + // module level: widened in the global initializer, after the writes before it + assert(moduleWidened.length == 4, "module: length"); + assert(moduleWidened[0] == 70, "module: written element"); + assert(moduleWidened[3] / 4 == 2.5, "module: pushed element, number arithmetic"); + + // module level: widened here, after a write made in this function + moduleArray[1] = 80; + const moduleLater: number[] = moduleArray; + assert(moduleLater[1] == 80, "module: written in function"); + + print("done."); +} diff --git a/tslang/test/tester/tests/00const_array_to_array_elements.ts b/tslang/test/tester/tests/00const_array_to_array_elements.ts index a28ba8f58..d2614b7e1 100644 --- a/tslang/test/tester/tests/00const_array_to_array_elements.ts +++ b/tslang/test/tester/tests/00const_array_to_array_elements.ts @@ -10,11 +10,8 @@ function lengthOf(u: number[] | string) { // `const c` case below, but through the global-variable codegen path (createGlobalVariable / // adjustGlobalVariableType), not the local one (createLocalVariable / adjustLocalVariableType). // Giving `const` array literals real identity storage (so mutating methods like .sort() share one -// heap array - see docs/const-let-storage-design.md) initially broke this: reading `moduleConst` -// back yields `AddressOf(@moduleConst) -> Load`, a shape castConstArrayToArray's local-only -// trace-back (Load -> VariableOp -> initializer) did not recognize, so it fell through to the -// generic array-to-array cast and was rejected ("element type number is not base of type s32"). -// Fixed by also unwrapping AddressOf(global) -> the GlobalOp's own initializer terminator. +// heap array - see docs/const-let-storage-design.md) means reading `moduleConst` back yields an +// `s32[]` loaded from the global, not the literal; castArrayElementwise converts what it holds. const moduleConst = [7, 8, 9]; function takesNumberArray(a: number[]) {