Skip to content

Commit 0d19d98

Browse files
sjrdsjrd
authored
Merge pull request #4898 from gzm0/honor-callsite-inline
Fix #1961: Honor call-site @inline / @noinline
2 parents 19d3ce0 + 6714397 commit 0d19d98

File tree

5 files changed

+241
-17
lines changed

5 files changed

+241
-17
lines changed

‎compiler/src/main/scala/org/scalajs/nscplugin/GenJSCode.scala

Lines changed: 34 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3263,6 +3263,15 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
32633263
val Apply(fun @ Select(receiver, _), args) = tree
32643264
val sym = fun.symbol
32653265

3266+
val inline = {
3267+
tree.hasAttachment[InlineCallsiteAttachment.type] ||
3268+
fun.hasAttachment[InlineCallsiteAttachment.type] // nullary methods
3269+
}
3270+
val noinline = {
3271+
tree.hasAttachment[NoInlineCallsiteAttachment.type] ||
3272+
fun.hasAttachment[NoInlineCallsiteAttachment.type] // nullary methods
3273+
}
3274+
32663275
if (isJSType(receiver.tpe) && sym.owner != ObjectClass) {
32673276
if (!isNonNativeJSClass(sym.owner) || isExposed(sym))
32683277
genPrimitiveJSCall(tree, isStat)
@@ -3278,38 +3287,47 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
32783287
*/
32793288
js.Skip()
32803289
} else {
3281-
genApplyStatic(sym, args.map(genExpr))
3290+
genApplyStatic(sym, args.map(genExpr), inline = inline, noinline = noinline)
32823291
}
32833292
} else {
32843293
genApplyMethodMaybeStatically(genExpr(receiver), sym,
3285-
genActualArgs(sym, args))
3294+
genActualArgs(sym, args), inline = inline, noinline = noinline)
32863295
}
32873296
}
32883297

32893298
def genApplyMethodMaybeStatically(receiver: js.Tree,
3290-
method: Symbol, arguments: List[js.Tree])(
3299+
method: Symbol, arguments: List[js.Tree],
3300+
inline: Boolean = false, noinline: Boolean = false)(
32913301
implicit pos: Position): js.Tree = {
32923302
if (method.isPrivate || method.isClassConstructor)
3293-
genApplyMethodStatically(receiver, method, arguments)
3303+
genApplyMethodStatically(receiver, method, arguments, inline = inline, noinline = noinline)
32943304
else
3295-
genApplyMethod(receiver, method, arguments)
3305+
genApplyMethod(receiver, method, arguments, inline = inline, noinline = noinline)
32963306
}
32973307

32983308
/** Gen JS code for a call to a Scala method. */
32993309
def genApplyMethod(receiver: js.Tree,
3300-
method: Symbol, arguments: List[js.Tree])(
3310+
method: Symbol, arguments: List[js.Tree],
3311+
inline: Boolean = false, noinline: Boolean = false)(
33013312
implicit pos: Position): js.Tree = {
33023313
assert(!method.isPrivate,
33033314
s"Cannot generate a dynamic call to private method $method at $pos")
3304-
js.Apply(js.ApplyFlags.empty, receiver, encodeMethodSym(method), arguments)(
3315+
val flags = js.ApplyFlags.empty
3316+
.withInline(inline)
3317+
.withNoinline(noinline)
3318+
3319+
js.Apply(flags, receiver, encodeMethodSym(method), arguments)(
33053320
toIRType(method.tpe.resultType))
33063321
}
33073322

33083323
def genApplyMethodStatically(receiver: js.Tree, method: Symbol,
3309-
arguments: List[js.Tree])(implicit pos: Position): js.Tree = {
3324+
arguments: List[js.Tree], inline: Boolean = false, noinline: Boolean = false)(
3325+
implicit pos: Position): js.Tree = {
33103326
val flags = js.ApplyFlags.empty
33113327
.withPrivate(method.isPrivate && !method.isClassConstructor)
33123328
.withConstructor(method.isClassConstructor)
3329+
.withInline(inline)
3330+
.withNoinline(noinline)
33133331
val methodIdent = encodeMethodSym(method)
33143332
val resultType =
33153333
if (method.isClassConstructor) jstpe.NoType
@@ -3323,11 +3341,15 @@ abstract class GenJSCode[G <: Global with Singleton](val global: G)
33233341
genApplyStatic(method, receiver :: arguments)
33243342
}
33253343

3326-
def genApplyStatic(method: Symbol, arguments: List[js.Tree])(
3344+
def genApplyStatic(method: Symbol, arguments: List[js.Tree],
3345+
inline: Boolean = false, noinline: Boolean = false)(
33273346
implicit pos: Position): js.Tree = {
3328-
js.ApplyStatic(js.ApplyFlags.empty.withPrivate(method.isPrivate),
3329-
encodeClassName(method.owner), encodeMethodSym(method), arguments)(
3330-
toIRType(method.tpe.resultType))
3347+
val flags = js.ApplyFlags.empty
3348+
.withPrivate(method.isPrivate)
3349+
.withInline(inline)
3350+
.withNoinline(noinline)
3351+
js.ApplyStatic(flags, encodeClassName(method.owner),
3352+
encodeMethodSym(method), arguments)(toIRType(method.tpe.resultType))
33313353
}
33323354

33333355
private def adaptPrimitive(value: js.Tree, to: jstpe.Type)(

‎compiler/src/test/scala/org/scalajs/nscplugin/test/CallSiteInlineTest.scala

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/*
2+
* Scala.js (https://www.scala-js.org/)
3+
*
4+
* Copyright EPFL.
5+
*
6+
* Licensed under Apache License 2.0
7+
* (https://www.apache.org/licenses/LICENSE-2.0).
8+
*
9+
* See the NOTICE file distributed with this work for
10+
* additional information regarding copyright ownership.
11+
*/
12+
13+
package org.scalajs.nscplugin.test
14+
15+
import util._
16+
17+
import org.junit.Test
18+
import org.junit.Assert._
19+
20+
import org.scalajs.ir.{Trees => js, Types => jstpe}
21+
import org.scalajs.ir.Names
22+
import org.scalajs.ir.Names._
23+
24+
class CallSiteInlineTest extends JSASTTest {
25+
object SMN {
26+
def unapply(ident: js.MethodIdent): Some[String] =
27+
Some(ident.name.simpleName.nameString)
28+
}
29+
30+
@Test
31+
def testInline: Unit = {
32+
val flags = {
33+
"""
34+
object A {
35+
println("F"): @inline
36+
}
37+
""".extractOne("println call") {
38+
case js.Apply(flags, _, SMN("println"), _) => flags
39+
}
40+
}
41+
42+
assertTrue(flags.inline)
43+
}
44+
45+
@Test
46+
def testNoinline: Unit = {
47+
val flags = {
48+
"""
49+
object A {
50+
println("F"): @noinline
51+
}
52+
""".extractOne("println call") {
53+
case js.Apply(flags, _, SMN("println"), _) => flags
54+
}
55+
}
56+
57+
assertTrue(flags.noinline)
58+
}
59+
60+
@Test
61+
def testInlineNullary: Unit = {
62+
val flags = {
63+
"""
64+
object A {
65+
Map.empty: @inline
66+
}
67+
""".extractOne("Map.empty") {
68+
case js.Apply(flags, _, SMN("empty"), _) => flags
69+
}
70+
}
71+
72+
assertTrue(flags.inline)
73+
}
74+
75+
@Test
76+
def testNoinlineNullary: Unit = {
77+
val flags = {
78+
"""
79+
object A {
80+
Map.empty: @noinline
81+
}
82+
""".extractOne("Map.empty") {
83+
case js.Apply(flags, _, SMN("empty"), _) => flags
84+
}
85+
}
86+
87+
assertTrue(flags.noinline)
88+
}
89+
90+
@Test
91+
def testInlineStatic: Unit = {
92+
val flags = {
93+
"""
94+
object A {
95+
Integer.compare(1, 2): @inline
96+
}
97+
""".extractOne("compare call") {
98+
case js.ApplyStatic(flags, _, SMN("compare"), _) => flags
99+
}
100+
}
101+
102+
assertTrue(flags.inline)
103+
}
104+
105+
@Test
106+
def testNoinlineStatic: Unit = {
107+
val flags = {
108+
"""
109+
object A {
110+
Integer.compare(1, 2): @noinline
111+
}
112+
""".extractOne("compare call") {
113+
case js.ApplyStatic(flags, _, SMN("compare"), _) => flags
114+
}
115+
}
116+
117+
assertTrue(flags.noinline)
118+
}
119+
}

‎ir/shared/src/main/scala/org/scalajs/ir/Trees.scala

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1299,13 +1299,25 @@ object Trees {
12991299

13001300
def isConstructor: Boolean = (bits & ConstructorBit) != 0
13011301

1302+
def inline: Boolean = (bits & InlineBit) != 0
1303+
1304+
def noinline: Boolean = (bits & NoinlineBit) != 0
1305+
13021306
def withPrivate(value: Boolean): ApplyFlags =
13031307
if (value) new ApplyFlags((bits & ~ConstructorBit) | PrivateBit)
13041308
else new ApplyFlags(bits & ~PrivateBit)
13051309

13061310
def withConstructor(value: Boolean): ApplyFlags =
13071311
if (value) new ApplyFlags((bits & ~PrivateBit) | ConstructorBit)
13081312
else new ApplyFlags(bits & ~ConstructorBit)
1313+
1314+
def withInline(value: Boolean): ApplyFlags =
1315+
if (value) new ApplyFlags(bits | InlineBit)
1316+
else new ApplyFlags(bits & ~InlineBit)
1317+
1318+
def withNoinline(value: Boolean): ApplyFlags =
1319+
if (value) new ApplyFlags(bits | NoinlineBit)
1320+
else new ApplyFlags(bits & ~NoinlineBit)
13091321
}
13101322

13111323
object ApplyFlags {
@@ -1315,6 +1327,12 @@ object Trees {
13151327
private final val ConstructorShift = 1
13161328
private final val ConstructorBit = 1 << ConstructorShift
13171329

1330+
private final val InlineShift = 2
1331+
private final val InlineBit = 1 << InlineShift
1332+
1333+
private final val NoinlineShift = 3
1334+
private final val NoinlineBit = 1 << NoinlineShift
1335+
13181336
final val empty: ApplyFlags =
13191337
new ApplyFlags(0)
13201338

‎linker/shared/src/main/scala/org/scalajs/linker/frontend/optimizer/OptimizerCore.scala

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1698,8 +1698,8 @@ private[optimizer] abstract class OptimizerCore(
16981698
else PreTransTree(Block(finishTransformStat(checked), Throw(Null())))
16991699
cont(checkedAndWellTyped)
17001700
case _ =>
1701-
if (methodName.isReflectiveProxy) {
1702-
// Never inline reflective proxies
1701+
if (methodName.isReflectiveProxy || flags.noinline) {
1702+
// Never inline reflective proxies or explicit noinlines.
17031703
treeNotInlined
17041704
} else {
17051705
val className = boxedClassForType(treceiver.tpe.base)
@@ -1880,7 +1880,7 @@ private[optimizer] abstract class OptimizerCore(
18801880
val targetMethod =
18811881
staticCall(className, MemberNamespace.forStaticCall(flags), method.name)
18821882

1883-
if (!targetMethod.attributes.inlineable) {
1883+
if (!targetMethod.attributes.inlineable || tree.flags.noinline) {
18841884
treeNotInlined
18851885
} else {
18861886
val maybeImportTarget = targetMethod.attributes.jsDynImportInlineTarget.orElse {
@@ -2309,9 +2309,14 @@ private[optimizer] abstract class OptimizerCore(
23092309

23102310
def default = {
23112311
val tall = optTReceiver.toList ::: targs
2312-
val shouldInline = target.attributes.inlineable && (
2312+
val shouldInline = {
2313+
target.attributes.inlineable &&
2314+
!flags.noinline && {
23132315
target.attributes.shouldInline ||
2314-
shouldInlineBecauseOfArgs(target, tall))
2316+
flags.inline ||
2317+
shouldInlineBecauseOfArgs(target, tall)
2318+
}
2319+
}
23152320

23162321
val allocationSites = tall.map(_.tpe.allocationSite)
23172322
val beingInlined = scope.implsBeingInlined((allocationSites, target))

‎linker/shared/src/test/scala/org/scalajs/linker/OptimizerTest.scala

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -561,6 +561,66 @@ class OptimizerTest {
561561
assertFalse(findClass(moduleSet, "Witness").isDefined)
562562
}
563563
}
564+
565+
def inlineFlagsTestCommon(optimizerHints: OptimizerHints, applyFlags: ApplyFlags,
566+
expectInline: Boolean): AsyncResult = await {
567+
val classDefs = Seq(
568+
classDef(
569+
MainTestClassName,
570+
kind = ClassKind.Class,
571+
superClass = Some(ObjectClass),
572+
methods = List(
573+
trivialCtor(MainTestClassName),
574+
MethodDef(EMF.withNamespace(PublicStatic), witnessMethodName, NON, Nil, AnyType, Some({
575+
// Non-trivial body to ensure no inlining by heuristics.
576+
Block(consoleLog(str("something")), str("result"))
577+
}))(optimizerHints, UNV),
578+
mainMethodDef({
579+
consoleLog({
580+
ApplyStatic(applyFlags, MainTestClassName, witnessMethodName, Nil)(AnyType)
581+
})
582+
})
583+
)
584+
)
585+
)
586+
587+
for {
588+
moduleSet <- linkToModuleSet(classDefs, MainTestModuleInitializers)
589+
} yield {
590+
val didInline = !findClass(moduleSet, MainTestClassName).get
591+
.methods.exists(_.name.name == witnessMethodName)
592+
593+
assertEquals(expectInline, didInline)
594+
}
595+
}
596+
597+
@Test
598+
def inlineFlagsTestDefault(): AsyncResult =
599+
inlineFlagsTestCommon(EOH, EAF, expectInline = false)
600+
601+
@Test
602+
def inlineFlagsTestInlineDefSite(): AsyncResult =
603+
inlineFlagsTestCommon(EOH.withInline(true), EAF, expectInline = true)
604+
605+
@Test
606+
def inlineFlagsTestNoinlineDefSite(): AsyncResult =
607+
inlineFlagsTestCommon(EOH.withNoinline(true), EAF, expectInline = false)
608+
609+
@Test
610+
def inlineFlagsTestInlineCallSite(): AsyncResult =
611+
inlineFlagsTestCommon(EOH, EAF.withInline(true), expectInline = true)
612+
613+
@Test
614+
def inlineFlagsTestNoinlineDefSiteNoOverride(): AsyncResult =
615+
inlineFlagsTestCommon(EOH.withNoinline(true), EAF.withInline(true), expectInline = false)
616+
617+
@Test
618+
def inlineFlagsTestNoinlineCallSite(): AsyncResult =
619+
inlineFlagsTestCommon(EOH, EAF.withNoinline(true), expectInline = false)
620+
621+
@Test
622+
def inlineFlagsTestNoinlineCallSiteOverride(): AsyncResult =
623+
inlineFlagsTestCommon(EOH.withInline(true), EAF.withNoinline(true), expectInline = false)
564624
}
565625

566626
object OptimizerTest {

0 commit comments

Comments
 (0)