Enable the IR checker post optimizer with RT longs

gzm0 · gzm0 · commit 256a2f4dfcfe · 2024-11-24T10:25:35.000+01:00
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.
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
@@ -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
@@ -774,10 +774,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.
@@ -6446,8 +6458,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
@@ -420,18 +420,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
@@ -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)
