From 0a620dbea753ac0d8c371d8d3471f95498c7f766 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Mon, 16 Dec 2024 11:02:33 +0100 Subject: [PATCH 1/5] Report duplicates in the IR after each pass --- .../org/enso/compiler/pass/PassManager.scala | 61 +++++++++++++++++-- 1 file changed, 57 insertions(+), 4 deletions(-) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala index 9c643deeb200..fa84a2aa930d 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala @@ -25,8 +25,9 @@ class PassManager( protected val passes: List[PassGroup], passConfiguration: PassConfiguration ) { - private val logger = LoggerFactory.getLogger(classOf[PassManager]) - val allPasses = verifyPassOrdering(passes.flatMap(_.passes)) + private val logger = LoggerFactory.getLogger(classOf[PassManager]) + val allPasses = verifyPassOrdering(passes.flatMap(_.passes)) + private val reported = new java.util.WeakHashMap[Any, Boolean]() /** Computes a valid pass ordering for the compiler. * @@ -165,6 +166,55 @@ class PassManager( ): IRType = { val pendingMiniPasses: ListBuffer[MiniPassFactory] = ListBuffer() + def checkDuplicates(msg: String, ir: IRType): IRType = { + def toString(ir: IR): String = { + ir.getClass().getName() + "@" + Integer.toHexString( + System.identityHashCode(ir) + ) + ":" + ir.showCode() + } + + val all = ir.preorder() + val dup = all + .groupBy(identity) + .collect { case (x, ys) if ys.lengthCompare(1) > 0 => x } + .filter { e => + if (reported.containsKey(e)) { + false + } else { + reported.put(e, true) + true + } + } + .toList + if (dup.nonEmpty) { + logger.error("Duplicates found after " + msg) + val map = new java.util.HashMap[IR, java.util.Set[IR]] + all.foreach(e => { + e.children.foreach(ch => { + if (dup.contains(ch)) { + var arr = map.get(ch) + if (arr == null) { + arr = new java.util.HashSet[IR] + map.put(ch, arr) + } + arr.add(e) + } + }) + }) + all.foreach(e => { + val refs = map.get(e) + if (refs != null && refs.size() > 1) { + logger.error(" " + toString(e) + " is referenced by") + refs.forEach { p => + logger.error(" " + toString(p)) + } + map.clear() + } + }) + } + ir + } + def flushMiniPasses(in: IRType): IRType = { if (pendingMiniPasses.nonEmpty) { val miniPasses = @@ -173,7 +223,9 @@ class PassManager( pendingMiniPasses.clear() if (combinedPass != null) { logger.trace(" flushing pending mini pass: {}", combinedPass) - miniPassCompile(combinedPass, in) + val res = miniPassCompile(combinedPass, in) + checkDuplicates("Flushing mini passes: " + combinedPass, res) + res } else { in } @@ -220,7 +272,8 @@ class PassManager( " mega running: {}", megaPass ) - megaPassCompile(megaPass, flushedIR, context) + val res = megaPassCompile(megaPass, flushedIR, context) + checkDuplicates("" + megaPass.getClass().getName(), res) } } From 0c214f6eabca4334317cc8c0232aa998de3fa413 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Mon, 16 Dec 2024 11:23:59 +0100 Subject: [PATCH 2/5] Rely on IR == when putting them into Map. Otherwise we burn too much CPU to finish the tests. --- .../org/enso/compiler/pass/PassManager.scala | 52 +++++++------------ 1 file changed, 19 insertions(+), 33 deletions(-) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala index fa84a2aa930d..5f77131daa52 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala @@ -27,7 +27,7 @@ class PassManager( ) { private val logger = LoggerFactory.getLogger(classOf[PassManager]) val allPasses = verifyPassOrdering(passes.flatMap(_.passes)) - private val reported = new java.util.WeakHashMap[Any, Boolean]() + private val reported = new java.util.HashMap[Integer, Boolean]() /** Computes a valid pass ordering for the compiler. * @@ -173,42 +173,28 @@ class PassManager( ) + ":" + ir.showCode() } - val all = ir.preorder() - val dup = all - .groupBy(identity) - .collect { case (x, ys) if ys.lengthCompare(1) > 0 => x } - .filter { e => - if (reported.containsKey(e)) { - false - } else { - reported.put(e, true) + val all = ir.preorder() + val idMap = new java.util.IdentityHashMap[Any, Boolean]() + val duplFound = all.find(e => { + if (reported.containsKey(System.identityHashCode(e))) { + false + } else { + if (idMap.containsKey(e)) { true + } else { + idMap.put(e, true) + false } } - .toList - if (dup.nonEmpty) { - logger.error("Duplicates found after " + msg) - val map = new java.util.HashMap[IR, java.util.Set[IR]] - all.foreach(e => { - e.children.foreach(ch => { - if (dup.contains(ch)) { - var arr = map.get(ch) - if (arr == null) { - arr = new java.util.HashSet[IR] - map.put(ch, arr) - } - arr.add(e) - } - }) - }) + }) + if (duplFound.nonEmpty) { + reported.put(System.identityHashCode(duplFound.orNull), true) + logger.error( + "Duplicate found after " + msg + ": " + toString(duplFound.orNull) + ) all.foreach(e => { - val refs = map.get(e) - if (refs != null && refs.size() > 1) { - logger.error(" " + toString(e) + " is referenced by") - refs.forEach { p => - logger.error(" " + toString(p)) - } - map.clear() + if (e.children().contains(duplFound.orNull)) { + logger.error(" referenced by " + toString(e)) } }) } From 6c4abd859b8b71b869f927ae4a6d276257e0f335 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Mon, 16 Dec 2024 14:37:22 +0100 Subject: [PATCH 3/5] Checking duplicates in IRHelpers --- .../org/enso/compiler/pass/IRHelpers.java | 79 +++++++++++++++++++ .../org/enso/compiler/pass/PassManager.scala | 47 ++--------- 2 files changed, 86 insertions(+), 40 deletions(-) create mode 100644 engine/runtime-compiler/src/main/java/org/enso/compiler/pass/IRHelpers.java diff --git a/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/IRHelpers.java b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/IRHelpers.java new file mode 100644 index 000000000000..a536eab89813 --- /dev/null +++ b/engine/runtime-compiler/src/main/java/org/enso/compiler/pass/IRHelpers.java @@ -0,0 +1,79 @@ +package org.enso.compiler.pass; + +import java.util.ArrayDeque; +import java.util.HashSet; +import java.util.IdentityHashMap; +import java.util.Set; +import org.enso.compiler.core.IR; +import org.enso.scala.wrapper.ScalaConversions; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +final class IRHelpers { + private static final Logger LOG = LoggerFactory.getLogger(IRHelpers.class); + private static HashSet reportedIdentityHashCodes; + + private IRHelpers() {} + + /** + * Processes whole IR subtree to find (top most) elements that are referenced multiple times. + * + * @param root the root of IR tree to process + * @return set of IR elements that appear at least twice + */ + private static Set findTopMostDuplicates(IR root) { + var foundIR = new IdentityHashMap(); + var irToProcess = new ArrayDeque(); + irToProcess.add(root); + while (!irToProcess.isEmpty()) { + var ir = irToProcess.remove(); + if (foundIR.containsKey(ir)) { + foundIR.put(ir, 1); + } else { + foundIR.put(ir, 0); + irToProcess.addAll(ScalaConversions.asJava(ir.children())); + } + } + var it = foundIR.entrySet().iterator(); + while (it.hasNext()) { + if (it.next().getValue() == 0) { + it.remove(); + } + } + return foundIR.keySet(); + } + + static IRType checkDuplicates(String msg, IRType ir) { + var duplicates = findTopMostDuplicates(ir); + if (duplicates.isEmpty()) { + return ir; + } else { + if (reportedIdentityHashCodes == null) { + reportedIdentityHashCodes = new HashSet<>(); + } + var all = ir.preorder(); + for (var dupl : duplicates) { + if (!reportedIdentityHashCodes.add(System.identityHashCode(dupl))) { + continue; + } + LOG.error("Duplicate found after " + msg + ": " + toString(dupl)); + all.foreach( + e -> { + if (e.children().contains(dupl)) { + LOG.error(" referenced by " + toString(e)); + } + return null; + }); + } + } + return ir; + } + + private static String toString(IR ir) { + return ir.getClass().getName() + + "@" + + Integer.toHexString(System.identityHashCode(ir)) + + ":" + + ir.showCode(); + } +} diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala index 5f77131daa52..08bc38d690df 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/PassManager.scala @@ -25,9 +25,8 @@ class PassManager( protected val passes: List[PassGroup], passConfiguration: PassConfiguration ) { - private val logger = LoggerFactory.getLogger(classOf[PassManager]) - val allPasses = verifyPassOrdering(passes.flatMap(_.passes)) - private val reported = new java.util.HashMap[Integer, Boolean]() + private val logger = LoggerFactory.getLogger(classOf[PassManager]) + val allPasses = verifyPassOrdering(passes.flatMap(_.passes)) /** Computes a valid pass ordering for the compiler. * @@ -166,41 +165,6 @@ class PassManager( ): IRType = { val pendingMiniPasses: ListBuffer[MiniPassFactory] = ListBuffer() - def checkDuplicates(msg: String, ir: IRType): IRType = { - def toString(ir: IR): String = { - ir.getClass().getName() + "@" + Integer.toHexString( - System.identityHashCode(ir) - ) + ":" + ir.showCode() - } - - val all = ir.preorder() - val idMap = new java.util.IdentityHashMap[Any, Boolean]() - val duplFound = all.find(e => { - if (reported.containsKey(System.identityHashCode(e))) { - false - } else { - if (idMap.containsKey(e)) { - true - } else { - idMap.put(e, true) - false - } - } - }) - if (duplFound.nonEmpty) { - reported.put(System.identityHashCode(duplFound.orNull), true) - logger.error( - "Duplicate found after " + msg + ": " + toString(duplFound.orNull) - ) - all.foreach(e => { - if (e.children().contains(duplFound.orNull)) { - logger.error(" referenced by " + toString(e)) - } - }) - } - ir - } - def flushMiniPasses(in: IRType): IRType = { if (pendingMiniPasses.nonEmpty) { val miniPasses = @@ -210,7 +174,10 @@ class PassManager( if (combinedPass != null) { logger.trace(" flushing pending mini pass: {}", combinedPass) val res = miniPassCompile(combinedPass, in) - checkDuplicates("Flushing mini passes: " + combinedPass, res) + IRHelpers.checkDuplicates( + "Flushing mini passes: " + combinedPass, + res + ) res } else { in @@ -259,7 +226,7 @@ class PassManager( megaPass ) val res = megaPassCompile(megaPass, flushedIR, context) - checkDuplicates("" + megaPass.getClass().getName(), res) + IRHelpers.checkDuplicates("" + megaPass.getClass().getName(), res) } } From bb4aff2531ffa973f3d988628334ae3a27cc4d62 Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Mon, 16 Dec 2024 14:45:58 +0100 Subject: [PATCH 4/5] Eliminate duplicated references to Main --- .../main/scala/org/enso/compiler/pass/desugar/Imports.scala | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/desugar/Imports.scala b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/desugar/Imports.scala index 51cfcc0588b2..1188afe76c96 100644 --- a/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/desugar/Imports.scala +++ b/engine/runtime-compiler/src/main/scala/org/enso/compiler/pass/desugar/Imports.scala @@ -50,7 +50,8 @@ case object Imports extends IRPass { val parts = newName.parts if (parts.length == 2) { i.copy( - name = newName.copy(parts = parts :+ mainModuleName), + name = + newName.copy(parts = parts :+ mainModuleName.duplicate()), rename = computeRename( i.rename, i.onlyNames.nonEmpty || i.isAll, From b4f3973034fc2f49dbc2abd2fe5bed7ad9efd9cb Mon Sep 17 00:00:00 2001 From: Jaroslav Tulach Date: Mon, 16 Dec 2024 15:48:52 +0100 Subject: [PATCH 5/5] Not enabled, when it throws exception --- .../org/enso/truffleloggerwrapper/TruffleLoggerWrapper.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/scala/logging-truffle-connector/src/main/scala/org/enso/truffleloggerwrapper/TruffleLoggerWrapper.scala b/lib/scala/logging-truffle-connector/src/main/scala/org/enso/truffleloggerwrapper/TruffleLoggerWrapper.scala index 603eee0b7302..fda39dcd8506 100644 --- a/lib/scala/logging-truffle-connector/src/main/scala/org/enso/truffleloggerwrapper/TruffleLoggerWrapper.scala +++ b/lib/scala/logging-truffle-connector/src/main/scala/org/enso/truffleloggerwrapper/TruffleLoggerWrapper.scala @@ -22,8 +22,11 @@ class TruffleLoggerWrapper(name: String, masking: Masking) extends Logger { override def getName: String = underlying.getName - private def isEnabled(level: Level): Boolean = + private def isEnabled(level: Level): Boolean = try { underlying.isLoggable(level) + } catch { + case _: IllegalStateException => false + } private def log( level: Level,