Skip to content

Commit

Permalink
Use subcolon args to simplify optimizer options
Browse files Browse the repository at this point in the history 
Grouping options `l:none`, `l:default`, `l:method`, `l:inline`,
are renamed `none`, `default`, `local`, `all`.

`-opt-inline-from:**` is dropped in favor of `-opt:inline:**`.

`-opt-warnings:_` is renamed `-Wopt:_`.

The ambiguity in parsing `-option:command:a,b,c`, where `-Wconf`
sees values `command:a`, `b`, `c`, is resolving by special-
casing `MultiChoiceSetting` only.
  • Loading branch information
@som-snytt
som-snytt committed Dec 14, 2021
1 parent e166296 commit ab29500
Show file tree
Hide file tree
Showing 68 changed files with 333 additions and 211 deletions.
1 change: 1 addition & 0 deletions build.sbt
View file
Expand Up @@ -682,6 +682,7 @@ lazy val bench = project.in(file("test") / "benchmarks")
if (benchmarkScalaVersion == "") Nil
else "org.scala-lang" % "scala-compiler" % benchmarkScalaVersion :: Nil
},
//scalacOptions ++= Seq("-feature", "-opt:all", "-opt:inline:scala/**", "-Wopt"),
scalacOptions ++= Seq("-feature", "-opt:l:inline", "-opt-inline-from:scala/**", "-opt-warnings"),
// Skips JMH source generators during IDE import to avoid needing to compile scala-library during the import
// should not be needed once sbt-jmh 0.4.3 is out (https://github.com/sbt/sbt-jmh/pull/207)
Expand Down
1 change: 1 addition & 0 deletions project/ScriptCommands.scala
View file
Expand Up @@ -161,6 +161,7 @@ object ScriptCommands {
}

private[this] val enableOptimizer = Seq(
//ThisBuild / Compile / scalacOptions ++= Seq("-opt:all", "-opt:inline:scala/**")
ThisBuild / Compile / scalacOptions ++= Seq("-opt:l:inline", "-opt-inline-from:scala/**")
)

Expand Down
2 changes: 1 addition & 1 deletion src/compiler/scala/tools/nsc/backend/jvm/PostProcessorFrontendAccess.scala
View file
Expand Up @@ -211,7 +211,7 @@ object PostProcessorFrontendAccess {
val optAllowSkipClassLoading: Boolean = s.optAllowSkipClassLoading

val optInlinerEnabled: Boolean = s.optInlinerEnabled
val optInlineFrom: List[String] = s.optInlineFrom.value
val optInlineFrom: List[String] = s.optInlineFrom
val optInlineHeuristics: String = s.YoptInlineHeuristics.value

val optWarningNoInlineMixed: Boolean = s.optWarningNoInlineMixed
Expand Down
110 changes: 67 additions & 43 deletions src/compiler/scala/tools/nsc/settings/MutableSettings.scala
View file
Expand Up @@ -122,11 +122,7 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
args: List[String],
setter: (Setting) => (List[String] => Option[List[String]])
): Option[List[String]] =
lookupSetting(cmd) match {
//case None => errorFn("Parameter '" + cmd + "' is not recognised by Scalac.") ; None
case None => None //error reported in processArguments
case Some(cmd) => setter(cmd)(args)
}
lookupSetting(cmd).flatMap(setter(_)(args))

// -Xfoo: clears Clearables
def clearIfExists(cmd: String): Option[List[String]] = lookupSetting(cmd) match {
Expand All @@ -139,14 +135,14 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
// the entire arg is consumed, so return None for failure
// any non-Nil return value means failure and we return s unmodified
def parseColonArg(s: String): Option[List[String]] =
if (s endsWith ":") {
if (s endsWith ":")
clearIfExists(s.init)
} else {
else
StringOps.splitWhere(s, _ == ':', doDropIndex = true).flatMap {
case (p, args) =>
tryToSetIfExists(p, (args split ",").toList, (s: Setting) => s.tryToSetColon(_))
// p:arg:a,b,c is taken as arg with selections a,b,c for a multichoice setting
case (p, args) if args.contains(":") && lookupSetting(p).map(_.isInstanceOf[MultiChoiceSetting[_]]).getOrElse(false) => tryToSetIfExists(p, List(args), (s: Setting) => s.tryToSetColon(_))
case (p, args) => tryToSetIfExists(p, args.split(",").toList, (s: Setting) => s.tryToSetColon(_))
}
}

// if arg is of form -Xfoo or -Xfoo bar (name = "-Xfoo")
def parseNormalArg(p: String, args: List[String]): Option[List[String]] =
Expand Down Expand Up @@ -559,7 +555,9 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
* not present in the multiChoiceSetting.value set, only their expansion.
*/
abstract class MultiChoiceEnumeration extends Enumeration {
case class Choice(name: String, help: String = "", expandsTo: List[Choice] = Nil) extends Val(name)
case class Choice(name: String, help: String = "", expandsTo: List[Choice] = Nil) extends Val(name) {
var selections: List[String] = Nil
}
def wildcardChoices: ValueSet = values.filter { case c: Choice => c.expandsTo.isEmpty case _ => true }
}

Expand Down Expand Up @@ -614,10 +612,10 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
private var sawAll = false

private def badChoice(s: String) = errorFn(s"'$s' is not a valid choice for '$name'")
private def isChoice(s: String) = (s == "_") || (choices contains pos(s))
private def isChoice(s: String) = s == "_" || choices.contains(pos(s))

private def pos(s: String) = s stripPrefix "-"
private def isPos(s: String) = !(s startsWith "-")
private def pos(s: String) = s.stripPrefix("-")
private def isPos(s: String) = !s.startsWith("-")

override val choices: List[String] = domain.values.toList map {
case ChoiceOrVal(name, _, _) => name
Expand All @@ -629,7 +627,7 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
case _ => ""
}

/** (Re)compute from current yeas, nays, wildcard status. */
/** (Re)compute from current yeas, nays, wildcard status. Assign option value. */
def compute() = {
def simple(v: domain.Value) = v match {
case c: domain.Choice => c.expandsTo.isEmpty
Expand All @@ -648,8 +646,8 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
}

// yeas from _ or expansions are weak: an explicit nay will disable them
val weakYeas = if (sawAll) domain.wildcardChoices else expand(yeas filterNot simple)
value = (yeas filter simple) | (weakYeas &~ nays)
val weakYeas = if (sawAll) domain.wildcardChoices else expand(yeas.filterNot(simple))
value = yeas.filter(simple) | (weakYeas &~ nays)
}

/** Add a named choice to the multichoice value. */
Expand All @@ -660,10 +658,10 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
sawAll = true
compute()
case _ if isPos(arg) =>
yeas += domain withName arg
yeas += domain.withName(arg)
compute()
case _ =>
val choice = domain withName pos(arg)
val choice = domain.withName(pos(arg))
choice match {
case ChoiceOrVal(_, _, _ :: _) => errorFn(s"'${pos(arg)}' cannot be negated, it enables other arguments")
case _ =>
Expand All @@ -672,6 +670,12 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
compute()
}

// refine a choice with selections. -opt:inline:**
def add(arg: String, selections: List[String]): Unit = {
add(arg)
domain.withName(arg).asInstanceOf[domain.Choice].selections ++= selections
}

def tryToSet(args: List[String]) = tryToSetArgs(args, halting = true)
override def tryToSetColon(args: List[String]) = tryToSetArgs(args, halting = false)
override def tryToSetFromPropertyValue(s: String) = tryToSet(s.trim.split(',').toList) // used from ide
Expand All @@ -680,37 +684,56 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
* The "halting" parameter means args were "-option a b c -else" so halt
* on "-else" or other non-choice. Otherwise, args were "-option:a,b,c,d",
* so process all and report non-choices as errors.
*
* If a choice is seen as colonated, then set the choice selections:
* "-option:choice:selection1,selection2"
*
* @param args args to process
* @param halting stop on non-arg
*/
private def tryToSetArgs(args: List[String], halting: Boolean) = {
val added = collection.mutable.ListBuffer.empty[String]

def tryArg(arg: String) = arg match {
case "help" => sawHelp = true
case s if isChoice(s) => added += s // this case also adds "_"
case s => badChoice(s)
}
@tailrec
def loop(args: List[String]): List[String] = args match {
case arg :: _ if halting && (!isPos(arg) || !isChoice(arg)) => args
case arg :: rest => tryArg(arg) ; loop(rest)
case Nil => Nil
}
val rest = loop(args)

// if no arg consumed, use defaults or error; otherwise, add what they added
if (rest.size == args.size) default match {
case Some(defaults) => defaults foreach add
case None => errorFn(s"'$name' requires an option. See '$name:help'.")
} else {
added foreach add
val colonnade = raw"([^:]+):(.*)".r
var count = 0
val rest = {
@tailrec
def loop(args: List[String]): List[String] = args match {
case "help" :: rest =>
sawHelp = true
count += 1
loop(rest)
case arg :: rest =>
val (argx, selections) = arg match {
case colonnade(x, y) => (x, y)
case _ => (arg, "")
}
if (halting && (!isPos(argx) || !isChoice(argx)))
args
else {
if (isChoice(argx)) {
if (selections.nonEmpty) add(argx, selections.split(",").toList)
else add(argx) // this case also adds "_"
postSetHook() // support -opt:l:method
}
else
badChoice(argx)
count += 1
loop(rest)
}
case _ => Nil
}
loop(args)
}

// if no arg applied, use defaults or error; otherwise, add what they added
if (count == 0)
default match {
case Some(defaults) => defaults.foreach(add)
case None => errorFn(s"'$name' requires an option. See '$name:help'.")
}
Some(rest)
}

def contains(choice: domain.Value): Boolean = value contains choice
def contains(choice: domain.Value): Boolean = value.contains(choice)

// programmatically.
def enable(choice: domain.Value): Unit = { nays -= choice ; yeas += choice ; compute() }
Expand All @@ -736,14 +759,15 @@ class MutableSettings(val errorFn: String => Unit, val pathFactory: PathFactory)
choices.zipAll(descriptions, "", "").map(describe).mkString(f"${descr}%n", f"%n", orelse)
}

def clear(): Unit = {
def clear(): Unit = {
domain.values.foreach { case c: domain.Choice => c.selections = Nil ; case _ => }
v = domain.ValueSet.empty
yeas = domain.ValueSet.empty
nays = domain.ValueSet.empty
sawAll = false
sawHelp = false
}
def unparse: List[String] = value.toList map (s => s"$name:$s")
def unparse: List[String] = value.toList.map(s => s"$name:$s")
def contains(s: String) = domain.values.find(_.toString == s).exists(value.contains)
}

Expand Down
103 changes: 70 additions & 33 deletions src/compiler/scala/tools/nsc/settings/ScalaSettings.scala
View file
Expand Up @@ -307,30 +307,30 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
val allowSkipCoreModuleInit = Choice("allow-skip-core-module-init", "Allow eliminating unused module loads for core modules of the standard library (e.g., Predef, ClassTag).")
val assumeModulesNonNull = Choice("assume-modules-non-null", "Assume loading a module never results in null (happens if the module is accessed in its super constructor).")
val allowSkipClassLoading = Choice("allow-skip-class-loading", "Allow optimizations that can skip or delay class loading.")
val inline = Choice("inline", "Inline method invocations according to -Yopt-inline-heuristics and -opt-inline-from.")
val inline = Choice("inline", "Inline method invocations from specified sites; also see -Yopt-inline-heuristics.")
val ell = Choice("l", "Deprecated l:none, l:default, l:method, l:inline.")

// l:none is not an expanding option, unlike the other l: levels. But it is excluded from -opt:_ below.
val lNone = Choice("l:none",
"Disable optimizations. Takes precedence: `-opt:l:none,+box-unbox` / `-opt:l:none -opt:box-unbox` don't enable box-unbox.")
// none is not an expanding option. It is excluded from -opt:_ below.
val lNone = Choice("none", "Disable all optimizations, including explicit options.")

private val defaultChoices = List(unreachableCode)
private val defaultOptimizations = List(unreachableCode)
val lDefault = Choice(
"l:default",
"Enable default optimizations: " + defaultChoices.mkString("", ",", "."),
expandsTo = defaultChoices)
"default",
defaultOptimizations.mkString("Enable default optimizations: ", ",", "."),
expandsTo = defaultOptimizations)

private val methodChoices = List(unreachableCode, simplifyJumps, compactLocals, copyPropagation, redundantCasts, boxUnbox, nullnessTracking, closureInvocations, allowSkipCoreModuleInit, assumeModulesNonNull, allowSkipClassLoading)
val localOptimizations = List(simplifyJumps, compactLocals, copyPropagation, redundantCasts, boxUnbox, nullnessTracking, closureInvocations, allowSkipCoreModuleInit, assumeModulesNonNull, allowSkipClassLoading)
val lMethod = Choice(
"l:method",
"Enable intra-method optimizations: " + methodChoices.mkString("", ",", "."),
expandsTo = methodChoices)
"local",
localOptimizations.mkString("Enable intra-method optimizations: ", ",", "."),
expandsTo = defaultOptimizations ::: localOptimizations)

private val inlineChoices = List(lMethod, inline)
val lInline = Choice("l:inline",
"Enable cross-method optimizations (note: inlining requires -opt-inline-from): " + inlineChoices.mkString("", ",", "."),
expandsTo = inlineChoices)
val lInline = Choice(
"all",
"Enable inlining, cross-method and local optimizations. To restrict inlining, use -opt:inline:sites as follows:\n" + inlineHelp,
expandsTo = inline :: defaultOptimizations ::: localOptimizations)

// "l:none" is excluded from wildcard expansion so that -opt:_ does not disable all settings
// "none" is excluded from wildcard expansion so that -opt:_ does not disable all settings
override def wildcardChoices = super.wildcardChoices.filter(_ ne lNone)
}

Expand All @@ -342,14 +342,26 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
helpArg = "optimization",
descr = "Enable optimizations, `help` for details.",
domain = optChoices,
)
).withPostSetHook { ss =>
// kludge alert: will be invoked twice, with selections available 2nd time
// for -opt:l:method reset the ell selections then enable local
if (ss.contains(optChoices.ell) && optChoices.ell.selections.nonEmpty) {
val todo = optChoices.ell.selections.map {
case "none" => "none"
case "default" => "default"
case "method" => "local"
case "inline" => "all"
}
optChoices.ell.selections = Nil
ss.tryToSetColon(todo)
}
}

private def optEnabled(choice: optChoices.Choice) = {
private def optEnabled(choice: optChoices.Choice) =
!opt.contains(optChoices.lNone) && {
opt.contains(choice) ||
!opt.isSetByUser && optChoices.lDefault.expandsTo.contains(choice)
}
}

def optNone = opt.contains(optChoices.lNone)
def optUnreachableCode = optEnabled(optChoices.unreachableCode)
Expand All @@ -369,11 +381,13 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
def optAddToBytecodeRepository = optBuildCallGraph || optInlinerEnabled || optClosureInvocations
def optUseAnalyzerCache = opt.isSetByUser && !optNone && (optBuildCallGraph || opt.value.size > 1)

val optInlineFrom = MultiStringSetting(
"-opt-inline-from",
"patterns",
"Patterns for classfile names from which to allow inlining, `help` for details.",
helpText = Some(
def optInlineFrom: List[String] =
optChoices.inline.selections match {
case Nil => List("**")
case sels => sels
}

def inlineHelp =
"""Patterns for classfile names from which the inliner is allowed to pull in code.
| * Matches classes in the empty package
| ** All classes
Expand All @@ -387,13 +401,25 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
| <sources> Classes defined in source files compiled in the current compilation, either
| passed explicitly to the compiler or picked up from the `-sourcepath`
|
|The setting accepts a list of patterns: `-opt-inline-from:p1,p2`. The setting can be passed
|The setting accepts a list of patterns: `-opt:inline:p1,p2`. The setting can be passed
|multiple times, the list of patterns gets extended. A leading `!` marks a pattern excluding.
|The last matching pattern defines whether a classfile is included or excluded (default: excluded).
|For example, `a.**,!a.b.**` includes classes in a and sub-packages, but not in a.b and sub-packages.
|
|Note: on the command-line you might need to quote patterns containing `*` to prevent the shell
|from expanding it to a list of files in the current directory.""".stripMargin))
|from expanding it to a list of files in the current directory.""".stripMargin

@deprecated("Deprecated alias", since="2.13.8")
val xoptInlineFrom = MultiStringSetting(
"-opt-inline-from",
"patterns",
"Patterns for classfile names from which to allow inlining, `help` for details.",
helpText = Some(inlineHelp))
//.withDeprecationMessage("use -opt:inline:**")
.withPostSetHook { from =>
opt.add("inline")
optChoices.inline.selections = from.value
}

val YoptInlineHeuristics = ChoiceSetting(
name = "-Yopt-inline-heuristics",
Expand All @@ -413,15 +439,26 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
}

val optWarnings = MultiChoiceSetting(
name = "-opt-warnings",
name = "-Wopt",
helpArg = "warning",
descr = "Enable optimizer warnings, `help` for details.",
domain = optWarningsChoices,
default = Some(List(optWarningsChoices.atInlineFailed.name))) withPostSetHook { _ =>
default = Some(List(optWarningsChoices.atInlineFailed.name))
).withPostSetHook { _ =>
// no need to set `Wconf` to `silent` if optWarnings is none, since no warnings are reported
if (optWarningsSummaryOnly) Wconf.tryToSet(List(s"cat=optimizer:ws"))
else Wconf.tryToSet(List(s"cat=optimizer:w"))
}
@deprecated("Deprecated alias", since="2.13.8")
val xoptWarnings = MultiChoiceSetting(
name = "-opt-warnings",
helpArg = "warning",
descr = "Enable optimizer warnings, `help` for details.",
domain = optWarningsChoices,
default = Some(List(optWarningsChoices.atInlineFailed.name))
).withPostSetHook { ow =>
optWarnings.value = ow.value
}//.withDeprecationMessage("Use -Wopt instead.")

def optWarningsSummaryOnly: Boolean = optWarnings.value subsetOf Set(optWarningsChoices.none, optWarningsChoices.atInlineFailedSummary)

Expand Down Expand Up @@ -519,11 +556,11 @@ trait ScalaSettings extends StandardScalaSettings with Warnings { _: MutableSett
val future = BooleanSetting("-Xfuture", "Replaced by -Xsource.").withDeprecationMessage("Not used since 2.13.")
val optimise = BooleanSetting("-optimize", "Enables optimizations.")
.withAbbreviation("-optimise")
.withDeprecationMessage("Since 2.12, enables -opt:l:inline -opt-inline-from:**. See -opt:help.")
.withPostSetHook(_ => {
.withDeprecationMessage("Since 2.12, enables -opt:inline:**. This can be dangerous.")
.withPostSetHook { _ =>
opt.enable(optChoices.lInline)
optInlineFrom.value = List("**")
})
optChoices.inline.selections = List("**")
}
val Xexperimental = BooleanSetting("-Xexperimental", "Former graveyard for language-forking extensions.")
.withDeprecationMessage("Not used since 2.13.")

Expand Down

0 comments on commit ab29500

Please sign in to comment.