Skip to content

Commit

Permalink
Merge pull request scala#8397 from retronym/backport/phase-order-regr…
Browse files Browse the repository at this point in the history
…ession

[backport] Backport phase assembly regression fix
  • Loading branch information
SethTisue authored Sep 4, 2019
2 parents f93c5c3 + f85b387 commit 3797ff9
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 116 deletions.
103 changes: 35 additions & 68 deletions src/compiler/scala/tools/nsc/PhaseAssembly.scala
Original file line number Diff line number Diff line change
Expand Up @@ -40,18 +40,14 @@ trait PhaseAssembly {
var phaseobj: Option[List[SubComponent]] = None
val after = new mutable.HashSet[Edge]()
var before = new mutable.HashSet[Edge]()
var visited: VisitStatus = NotVisited
var visited = false
var level = 0

def allPhaseNames(): String = phaseobj match {
case None => phasename
case Some(lst) => lst.map(_.phaseName).reduceLeft(_+","+_)
}
}
sealed abstract class VisitStatus
case object NotVisited extends VisitStatus
case object Visiting extends VisitStatus
case object Visited extends VisitStatus

val nodes = new mutable.HashMap[String,Node]()
val edges = new mutable.HashSet[Edge]()
Expand Down Expand Up @@ -108,69 +104,45 @@ trait PhaseAssembly {
/* Test if there are cycles in the graph, assign levels to the nodes
* and collapse hard links into nodes
*/
def collapseHardLinks() {
for (node <- nodes.valuesIterator.toList) {
val hardBefores = node.before.iterator.filter(_.hard).toList
for (hl <- hardBefores) {
val effectiveNode: Node = if (nodes.contains(node.name)) node else {
nodes.find(_._2.phaseobj.exists(_.exists(_.phaseName == node.name))).get._2
}
effectiveNode.phaseobj = Some(effectiveNode.phaseobj.get ++ hl.frm.phaseobj.get)
effectiveNode.before = hl.frm.before
nodes -= hl.frm.phasename
edges -= hl
}
def collapseHardLinksAndLevels(node: Node, lvl: Int) {
if (node.visited) {
dump("phase-cycle")
throw new FatalError(s"Cycle in phase dependencies detected at ${node.phasename}, created phase-cycle.dot")
}
}

/* Test if there are cycles in the graph, assign levels to the nodes
* and collapse hard links into nodes
*/
def assignLevelsAndDetectCycles(node: Node) {
val stack = mutable.ArrayStack[Node]()
def visitNode(node: Node): Unit = {
node.visited = Visiting
for (edge <- node.before) {
val from = edge.frm
from.visited match {
case NotVisited =>
visitNode(edge.frm)
case Visiting =>
dump("phase-cycle")
throw new FatalError(s"Cycle in phase dependencies detected at ${node.phasename}, created phase-cycle.dot")
case Visited =>
}
val initLevel = node.level
val levelUp = initLevel < lvl
if (levelUp) {
node.level = lvl
}
if (initLevel != 0) {
if (!levelUp) {
// no need to revisit
node.visited = false
return
}
node.visited = Visited
stack.push(node)
}
try {
visitNode(node)
} finally {
nodes.values.foreach(_.visited = NotVisited)
var befores = node.before
def hasHardLinks() = befores.exists(_.hard)
while (hasHardLinks()) {
for (hl <- befores) {
if (hl.hard) {
node.phaseobj = Some(node.phaseobj.get ++ hl.frm.phaseobj.get)
node.before = hl.frm.before
nodes -= hl.frm.phasename
edges -= hl
for (edge <- node.before) edge.to = node
}
}
befores = node.before
}
node.visited = true

val topoSort: Map[Node, Int] = stack.zipWithIndex.toMap
val root = node
assert(stack.head == root, stack)
root.level = 1

// Nodes that have been collapsed into their hard-linked predecessor
val collapsed: Map[String, Node] = stack.iterator.flatMap(p => p.phaseobj.toList.flatMap(_.map(x => (x.phaseName, p)))).toMap
def followHard(node: Node): Node = collapsed.getOrElse(node.phasename, node)

// find the longest path to the root node to assign as the level.
stack.iterator.drop(1).foreach { node =>
var n = node
var level = 1
while (n != root && n.after.nonEmpty) {
n = n.after.maxBy(edge => topoSort.get(followHard(edge.to))).to
level += 1
}
node.level = level
for (edge <- node.before) {
collapseHardLinksAndLevels( edge.frm, lvl + 1)
}
def phaseDebug = stack.toSeq.groupBy(_.level).toList.sortBy(_._1).map { case (level, nodes) => (level, nodes.sortBy(_.phasename).map(_.phaseobj.map(_.map(_.phaseName).mkString(":")).mkString(" ")).mkString(" "))}.mkString("\n")
debuglog("Phase assembly levels: " + phaseDebug)

node.visited = false
}

/* Find all edges in the given graph that are hard links. For each hard link we
Expand Down Expand Up @@ -263,16 +235,11 @@ trait PhaseAssembly {

dump(3)

// collapse hard links into nodes
graph.collapseHardLinks()
// test for cycles, assign levels and collapse hard links into nodes
graph.collapseHardLinksAndLevels(graph.getNodeByPhase("parser"), 1)

dump(4)

// test for cycles, assign levels
graph.assignLevelsAndDetectCycles(graph.getNodeByPhase("parser"))

dump(5)

// assemble the compiler
graph.compilerPhaseList()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ class PhaseAssemblyBenchmark {
var data: Data[_] = _

@Param(Array("1", "4", "8", "16"))
var size: Int = 16
var size: Int = 0

@Setup
def setup(): Unit = {
Expand Down Expand Up @@ -47,8 +47,7 @@ class PhaseAssemblyBenchmark {
val g = s.global
val graph = g.phasesSetToDepGraph(s.components.reverse)
graph.removeDanglingNodes()
graph.collapseHardLinks()
graph.assignLevelsAndDetectCycles(graph.getNodeByPhase("parser"))
graph.collapseHardLinksAndLevels(graph.getNodeByPhase("parser"), 1)
graph
}
}
Expand Down
2 changes: 1 addition & 1 deletion test/files/neg/t7622-cyclic-dependency.check
Original file line number Diff line number Diff line change
@@ -1 +1 @@
error: Cycle in phase dependencies detected at cyclicdependency2, created phase-cycle.dot
error: Cycle in phase dependencies detected at cyclicdependency1, created phase-cycle.dot
44 changes: 0 additions & 44 deletions test/junit/scala/tools/nsc/PhaseAssemblyTest.scala

This file was deleted.

0 comments on commit 3797ff9

Please sign in to comment.