From fdd2be3a338fc9ae8a2689ef61c229cd149be099 Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Sun, 17 Nov 2024 10:40:41 +0100 Subject: [PATCH 1/4] Optimizer: Do not expand long ops if we cannot inline them --- .../frontend/optimizer/OptimizerCore.scala | 46 +++++++++---------- 1 file changed, 21 insertions(+), 25 deletions(-) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala index dae5f87e43..a77967b134 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala @@ -3531,34 +3531,30 @@ private[optimizer] abstract class OptimizerCore( implicit scope: Scope): TailRec[Tree] = { implicit val pos = pretrans.pos - // unfortunately nullable for the result types of methods - def rtLongClassType = ClassType(LongImpl.RuntimeLongClass, nullable = true) + def default = cont(pretrans) def expandLongModuleOp(methodName: MethodName, arg: PreTransform): TailRec[Tree] = { import LongImpl.{RuntimeLongModuleClass => modCls} - val receiver = + val treceiver = makeCast(LoadModule(modCls), ClassType(modCls, nullable = false)).toPreTransform - pretransformApply(ApplyFlags.empty, receiver, MethodIdent(methodName), - arg :: Nil, rtLongClassType, isStat = false, - usePreTransform = true)( - cont) + val impl = staticCall(modCls, MemberNamespace.Public, methodName) + pretransformSingleDispatch(ApplyFlags.empty, impl, Some(treceiver), arg :: Nil, + isStat = false, usePreTransform = true)(cont)(default) } - def expandUnaryOp(methodName: MethodName, arg: PreTransform, - resultType: Type = rtLongClassType): TailRec[Tree] = { - pretransformApply(ApplyFlags.empty, arg, MethodIdent(methodName), Nil, - resultType, isStat = false, usePreTransform = true)( - cont) + def expandLongClassOp(methodName: MethodName, receiver: PreTransform, + args: List[PreTransform]): TailRec[Tree] = { + val impl = staticCall(LongImpl.RuntimeLongClass, MemberNamespace.Public, methodName) + pretransformSingleDispatch(ApplyFlags.empty, impl, Some(receiver), args, + isStat = false, usePreTransform = true)(cont)(default) } - def expandBinaryOp(methodName: MethodName, lhs: PreTransform, - rhs: PreTransform, - resultType: Type = rtLongClassType): TailRec[Tree] = { - pretransformApply(ApplyFlags.empty, lhs, MethodIdent(methodName), rhs :: Nil, - resultType, isStat = false, usePreTransform = true)( - cont) - } + def expandUnaryOp(methodName: MethodName, arg: PreTransform) = + expandLongClassOp(methodName, arg, Nil) + + def expandBinaryOp(methodName: MethodName, lhs: PreTransform, rhs: PreTransform) = + expandLongClassOp(methodName, lhs, rhs :: Nil) pretrans match { case PreTransUnaryOp(op, arg) if useRuntimeLong => @@ -3569,19 +3565,19 @@ private[optimizer] abstract class OptimizerCore( expandLongModuleOp(LongImpl.fromInt, arg) case LongToInt => - expandUnaryOp(LongImpl.toInt, arg, IntType) + expandUnaryOp(LongImpl.toInt, arg) case LongToDouble => - expandUnaryOp(LongImpl.toDouble, arg, DoubleType) + expandUnaryOp(LongImpl.toDouble, arg) case DoubleToLong => expandLongModuleOp(LongImpl.fromDouble, arg) case LongToFloat => - expandUnaryOp(LongImpl.toFloat, arg, FloatType) + expandUnaryOp(LongImpl.toFloat, arg) case _ => - cont(pretrans) + default } case PreTransBinaryOp(op, lhs, rhs) if useRuntimeLong => @@ -3618,11 +3614,11 @@ private[optimizer] abstract class OptimizerCore( case Long_>= => expandBinaryOp(LongImpl.>=, lhs, rhs) case _ => - cont(pretrans) + default } case _ => - cont(pretrans) + default } } From be9fc38f1ae68f79ef3986433171d1e12663504e Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Sun, 17 Nov 2024 12:46:05 +0100 Subject: [PATCH 2/4] Optimizer: Fail if we cannot inline RuntimeLong --- .../linker/frontend/optimizer/OptimizerCore.scala | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala index a77967b134..2c520cf8e9 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala @@ -3519,11 +3519,15 @@ private[optimizer] abstract class OptimizerCore( withBinding(rtLongBinding) { (scope1, cont1) => implicit val scope = scope1 val tRef = VarRef(tName)(rtLongClassType) - val newTree = New(LongImpl.RuntimeLongClass, - MethodIdent(LongImpl.initFromParts), - List(Apply(ApplyFlags.empty, tRef, MethodIdent(LongImpl.lo), Nil)(IntType), - Apply(ApplyFlags.empty, tRef, MethodIdent(LongImpl.hi), Nil)(IntType))) - pretransformExpr(newTree)(cont1) + + val lo = Apply(ApplyFlags.empty, tRef, MethodIdent(LongImpl.lo), Nil)(IntType) + val hi = Apply(ApplyFlags.empty, tRef, MethodIdent(LongImpl.hi), Nil)(IntType) + + pretransformExprs(lo, hi) { (tlo, thi) => + inlineClassConstructor(AllocationSite.Anonymous, LongImpl.RuntimeLongClass, + inlinedRTLongStructure, MethodIdent(LongImpl.initFromParts), List(tlo, thi), + () => throw new AssertionError(s"rolled-back RuntimeLong inlining at $pos"))(cont1) + } } (cont) } From 84aec56444dd01d18aa27823d52165d901c46252 Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Sun, 24 Nov 2024 10:22:38 +0100 Subject: [PATCH 3/4] Optimizer: Make special subtyping checks for RuntimeLong explicit To enable the IRChecker, we only want to consider RuntimeLong equivalent to BoxedLong when we'll end up removing that type information from the IR. In practice, this is during instance checks. --- .../frontend/optimizer/OptimizerCore.scala | 39 ++++++++----------- 1 file changed, 17 insertions(+), 22 deletions(-) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala index 2c520cf8e9..a167950d4e 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala @@ -262,27 +262,8 @@ private[optimizer] abstract class OptimizerCore( private val isSubclassFun = isSubclass _ - private def isSubtype(lhs: Type, rhs: Type): Boolean = { - assert(lhs != VoidType) - assert(rhs != VoidType) - - Types.isSubtype(lhs, rhs)(isSubclassFun) || { - (lhs, rhs) match { - case (LongType, ClassType(LongImpl.RuntimeLongClass, _)) => - true - case (ClassType(BoxedLongClass, lhsNullable), - ClassType(LongImpl.RuntimeLongClass, rhsNullable)) => - rhsNullable || !lhsNullable - - case (ClassType(LongImpl.RuntimeLongClass, lhsNullable), - ClassType(BoxedLongClass, rhsNullable)) => - rhsNullable || !lhsNullable - - case _ => - false - } - } - } + private def isSubtype(lhs: Type, rhs: Type): Boolean = + Types.isSubtype(lhs, rhs)(isSubclassFun) /** Transforms a statement. * @@ -565,8 +546,22 @@ private[optimizer] abstract class OptimizerCore( case IsInstanceOf(expr, testType) => trampoline { pretransformExpr(expr) { texpr => + val texprType = texpr.tpe.base.toNonNullable + + def isLongType(tpe: Type) = tpe match { + case LongType => true + case ClassType(LongImpl.RuntimeLongClass | BoxedLongClass, _) => true + case _ => false + } + + // Note: Disregards nullability because we can optimize null-check only. + val staticSubtype = { + isSubtype(texprType, testType) || + (useRuntimeLong && isLongType(testType) && isLongType(texprType)) + } + val result = { - if (isSubtype(texpr.tpe.base.toNonNullable, testType)) { + if (staticSubtype) { if (texpr.tpe.isNullable) BinaryOp(BinaryOp.!==, finishTransformExpr(texpr), Null()) else From eb41c0200556d394bc7a142d429a944592aec6a2 Mon Sep 17 00:00:00 2001 From: Tobias Schlatter Date: Sun, 24 Nov 2024 10:25:35 +0100 Subject: [PATCH 4/4] Enable the IR checker post optimizer with RT longs To keep the IR checker happy, we insert casts at method boundaries that cast RuntimeLongs back to LongType. The first two commits in this PR are not strictly necessary. However, I felt they significantly help understanding of what the optimizer is doing. --- .../org/scalajs/linker/frontend/Refiner.scala | 12 +-------- .../frontend/optimizer/OptimizerCore.scala | 20 +++++++++++--- .../org/scalajs/linker/IRCheckerTest.scala | 10 +------ .../org/scalajs/linker/OptimizerTest.scala | 27 +++++++++++++++++++ 4 files changed, 45 insertions(+), 24 deletions(-) diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/Refiner.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/Refiner.scala index a263a7b388..075a700f52 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/Refiner.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/Refiner.scala @@ -34,16 +34,6 @@ final class Refiner(config: CommonPhaseConfig, checkIR: Boolean) { private val analyzer = new Analyzer(config, initial = false, checkIR = checkIR, failOnError = true, irLoader) - /* TODO: Remove this and replace with `checkIR` once the optimizer generates - * well-typed IR with runtime longs. - */ - private val shouldRunIRChecker = { - val optimizerUsesRuntimeLong = - !config.coreSpec.esFeatures.allowBigIntsForLongs && - !config.coreSpec.targetIsWebAssembly - checkIR && !optimizerUsesRuntimeLong - } - def refine(classDefs: Seq[(ClassDef, Version)], moduleInitializers: List[ModuleInitializer], symbolRequirements: SymbolRequirement, logger: Logger)( @@ -77,7 +67,7 @@ final class Refiner(config: CommonPhaseConfig, checkIR: Boolean) { linkedTopLevelExports.flatten.toList, moduleInitializers, globalInfo) } - if (shouldRunIRChecker) { + if (checkIR) { logger.time("Refiner: Check IR") { val errorCount = IRChecker.check(result, logger, postOptimizer = true) if (errorCount != 0) { diff --git a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala index a167950d4e..b3057060eb 100644 --- a/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala +++ b/linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala @@ -743,10 +743,22 @@ private[optimizer] abstract class OptimizerCore( def addCaptureParam(newName: LocalName): LocalDef = { val newOriginalName = originalNameForFresh(paramName, originalName, newName) + val captureTpe = { + /* Do not refine the capture type for longs: + * The pretransform might be a stack allocated RuntimeLong. + * We cannot (trivially) capture it in stack allocated form. + * Therefore, we keep the primitive type and let finishTransformExpr + * allocate a RuntimeLong. + * TODO: Improve this and allocate two capture params for lo/hi? + */ + if (useRuntimeLong && paramDef.ptpe == LongType) RefinedType(LongType) + else tcaptureValue.tpe + } + val replacement = ReplaceWithVarRef(newName, newSimpleState(Unused)) - val localDef = LocalDef(tcaptureValue.tpe, mutable, replacement) + val localDef = LocalDef(captureTpe, mutable, replacement) val localIdent = LocalIdent(newName)(ident.pos) - val newParamDef = ParamDef(localIdent, newOriginalName, tcaptureValue.tpe.base, mutable)(paramDef.pos) + val newParamDef = ParamDef(localIdent, newOriginalName, captureTpe.base, mutable)(paramDef.pos) /* Note that the binding will never create a fresh name for a * ReplaceWithVarRef. So this will not put our name alignment at risk. @@ -6383,8 +6395,8 @@ private[optimizer] object OptimizerCore { private def createNewLong(lo: Tree, hi: Tree)( implicit pos: Position): Tree = { - New(LongImpl.RuntimeLongClass, MethodIdent(LongImpl.initFromParts), - List(lo, hi)) + makeCast(New(LongImpl.RuntimeLongClass, MethodIdent(LongImpl.initFromParts), + List(lo, hi)), LongType) } /** Tests whether `x + y` is valid without falling out of range. */ diff --git a/linker/shared/src/test/scala/org/scalajs/linker/IRCheckerTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/IRCheckerTest.scala index d283ae189e..019b78399e 100644 --- a/linker/shared/src/test/scala/org/scalajs/linker/IRCheckerTest.scala +++ b/linker/shared/src/test/scala/org/scalajs/linker/IRCheckerTest.scala @@ -451,18 +451,10 @@ object IRCheckerTest { moduleInitializers: List[ModuleInitializer], logger: Logger, postOptimizer: Boolean)( implicit ec: ExecutionContext): Future[Unit] = { - val baseConfig = StandardConfig() + val config = StandardConfig() .withCheckIR(true) .withOptimizer(false) - val config = { - /* Disable RuntimeLongs to workaround the Refiner disabling IRChecks in this case. - * TODO: Remove once we run IRChecks post optimizer all the time. - */ - if (postOptimizer) baseConfig.withESFeatures(_.withAllowBigIntsForLongs(true)) - else baseConfig - } - val noSymbolRequirements = SymbolRequirement .factory("IRCheckerTest") .none() diff --git a/linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala b/linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala index 3954dfe278..9d12a0d2eb 100644 --- a/linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala +++ b/linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala @@ -487,6 +487,33 @@ class OptimizerTest { } } + @Test + def testLongCaptures(): AsyncResult = await { + val calc = m("calc", Nil, LongRef) + + val classDefs = Seq( + classDef( + MainTestClassName, + kind = ClassKind.Class, + superClass = Some(ObjectClass), + methods = List( + // @noinline static def calc(): Long = 1L + MethodDef(EMF.withNamespace(PublicStatic), calc, NON, Nil, + LongType, Some(LongLiteral(1)))(EOH.withNoinline(true), UNV), + mainMethodDef(Block( + VarDef("x", NON, LongType, mutable = false, + ApplyStatic(EAF, MainTestClassName, calc, Nil)(LongType)), + consoleLog(Closure(true, List(paramDef("y", LongType)), Nil, None, + VarRef("y")(LongType), List(VarRef("x")(LongType)))) + )) + ) + ) + ) + + // Check it doesn't fail IRChecking. + linkToModuleSet(classDefs, MainTestModuleInitializers) + } + private def commonClassDefsForFieldRemovalTests(classInline: Boolean, witnessMutable: Boolean): Seq[ClassDef] = { val methodName = m("method", Nil, I)