Fix inliner to make inlining test pass.
This does not address the issue with inlining constructors on
Dalvik 4.4.4, yet.
Bug: 65355452
Change-Id: I548b4e49df1f67c833545905f500891edfc07624
diff --git a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
index e7f3f70..84d581d 100644
--- a/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
+++ b/src/main/java/com/android/tools/r8/graph/DexEncodedMethod.java
@@ -123,23 +123,34 @@
return accessFlags.isConstructor() && accessFlags.isStatic();
}
+ /**
+ * Returns true if this method can be invoked via invoke-virtual, invoke-super or
+ * invoke-interface.
+ */
public boolean isVirtualMethod() {
return !accessFlags.isStatic() && !accessFlags.isPrivate() && !accessFlags.isConstructor();
}
+ /**
+ * Returns true if this method can be invoked via invoke-virtual, invoke-super or invoke-interface
+ * and is non-abstract.
+ */
public boolean isNonAbstractVirtualMethod() {
- return !accessFlags.isStatic()
- && !accessFlags.isPrivate()
- && !accessFlags.isConstructor()
- && !accessFlags.isAbstract();
+ return isVirtualMethod() && !accessFlags.isAbstract();
}
+ /**
+ * Returns true if this method can be invoked via invoke-direct.
+ */
public boolean isDirectMethod() {
- return accessFlags.isPrivate() && !accessFlags.isStatic();
+ return (accessFlags.isPrivate() || accessFlags.isConstructor()) && !accessFlags.isStatic();
}
+ /**
+ * Returns true if this method can be invoked via invoke-static.
+ */
public boolean isStaticMethod() {
- return accessFlags.isStatic() || accessFlags.isConstructor();
+ return accessFlags.isStatic();
}
diff --git a/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java b/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java
index 12040e1..0c081c5 100644
--- a/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java
+++ b/src/main/java/com/android/tools/r8/ir/code/InvokeMethod.java
@@ -107,7 +107,7 @@
// We also have to take the constraint of the enclosing class into account.
Constraint classConstraint = Constraint
.deriveConstraint(holder, methodHolder, methodClass.accessFlags, info);
- result = Constraint.min(methodConstraint, classConstraint);
+ result = Constraint.min(result, classConstraint);
}
if (result == Constraint.NEVER) {
break;
diff --git a/src/test/examples/inlining/A.java b/src/test/examples/inlining/A.java
new file mode 100644
index 0000000..dd857c8
--- /dev/null
+++ b/src/test/examples/inlining/A.java
@@ -0,0 +1,25 @@
+// Copyright (c) 2017, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package inlining;
+
+class A {
+
+ int a;
+
+ A(int a) {
+ this.a = a;
+ }
+
+ int a() {
+ return a;
+ }
+
+ int cannotInline(int v) {
+ // Cannot inline due to recursion.
+ if (v > 0) {
+ return cannotInline(v - 1);
+ }
+ return 42;
+ }
+}
diff --git a/src/test/examples/inlining/B.java b/src/test/examples/inlining/B.java
new file mode 100644
index 0000000..0628187
--- /dev/null
+++ b/src/test/examples/inlining/B.java
@@ -0,0 +1,19 @@
+// Copyright (c) 2017, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package inlining;
+
+class B extends A {
+
+ B(int a) {
+ super(a);
+ }
+
+ int cannotInline(int v) {
+ return -1;
+ }
+
+ int callMethodInSuper() {
+ return super.cannotInline(10);
+ }
+}
diff --git a/src/test/examples/inlining/InlineConstructor.java b/src/test/examples/inlining/InlineConstructor.java
new file mode 100644
index 0000000..382d0c7
--- /dev/null
+++ b/src/test/examples/inlining/InlineConstructor.java
@@ -0,0 +1,49 @@
+// Copyright (c) 2017, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package inlining;
+
+class InlineConstructor {
+
+ int a;
+
+ @CheckDiscarded
+ InlineConstructor(int a) {
+ this.a = a;
+ }
+
+ InlineConstructor(long a) {
+ this((int) a);
+ }
+
+ InlineConstructor(int a, int loopy) {
+ this.a = a;
+ // Make this too big to inline.
+ if (loopy > 10) {
+ throw new RuntimeException("Too big!");
+ }
+ for (int i = 1; i < loopy; i++) {
+ this.a = this.a * i;
+ }
+ }
+
+ // TODO(b/65355452): This should be inlined.
+ // @CheckDiscarded
+ InlineConstructor() {
+ this(42, 9);
+ }
+
+ static InlineConstructor create() {
+ return new InlineConstructor(10L);
+ }
+
+ static InlineConstructor createMore() {
+ new InlineConstructor(0, 0);
+ new InlineConstructor(0, 0);
+ new InlineConstructor(0, 0);
+ new InlineConstructor(0, 0);
+ new InlineConstructor(0, 0);
+ new InlineConstructor(0, 0);
+ return new InlineConstructor();
+ }
+}
diff --git a/src/test/examples/inlining/InlineConstructorOfInner.java b/src/test/examples/inlining/InlineConstructorOfInner.java
new file mode 100644
index 0000000..cec7d3f
--- /dev/null
+++ b/src/test/examples/inlining/InlineConstructorOfInner.java
@@ -0,0 +1,34 @@
+// Copyright (c) 2017, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package inlining;
+
+class InlineConstructorOfInner {
+
+ class Inner {
+
+ int a;
+
+ @CheckDiscarded
+ Inner(int a) {
+ this.a = a;
+ }
+
+ // This is not inlined, even though it is only called once, as it is only called from a
+ // non-constructor, and will set a field (the outer object) before calling the other
+ // constructor.
+ Inner(long a) {
+ this((int) a);
+ }
+
+ public Inner create() {
+ return new Inner(10L);
+ }
+ }
+
+ Inner inner;
+
+ InlineConstructorOfInner() {
+ inner = new Inner(10L).create();
+ }
+}
diff --git a/src/test/examples/inlining/Inlining.java b/src/test/examples/inlining/Inlining.java
index 02a4b7c..c72812c 100644
--- a/src/test/examples/inlining/Inlining.java
+++ b/src/test/examples/inlining/Inlining.java
@@ -7,116 +7,6 @@
import inlining.pkg.PublicClass;
import inlining.pkg.Subclass;
-class A {
-
- int a;
-
- A(int a) {
- this.a = a;
- }
-
- int a() {
- return a;
- }
-
- int cannotInline(int v) {
- // Cannot inline due to recursion.
- if (v > 0) {
- return cannotInline(v - 1);
- }
- return 42;
- }
-}
-
-class B extends A {
-
- B(int a) {
- super(a);
- }
-
- int cannotInline(int v) {
- return -1;
- }
-
- int callMethodInSuper() {
- return super.cannotInline(10);
- }
-}
-
-class InlineConstructor {
-
- int a;
-
- @CheckDiscarded
- InlineConstructor(int a) {
- this.a = a;
- }
-
- InlineConstructor(long a) {
- this((int) a);
- }
-
- InlineConstructor(int a, int loopy) {
- this.a = a;
- // Make this too big to inline.
- if (loopy > 10) {
- throw new RuntimeException("Too big!");
- }
- for (int i = 1; i < loopy; i++) {
- this.a = this.a * i;
- }
- }
-
- @CheckDiscarded
- InlineConstructor() {
- this(42, 9);
- }
-
- static InlineConstructor create() {
- return new InlineConstructor(10L);
- }
-
- static InlineConstructor createMore() {
- new InlineConstructor(0, 0);
- new InlineConstructor(0, 0);
- new InlineConstructor(0, 0);
- new InlineConstructor(0, 0);
- new InlineConstructor(0, 0);
- new InlineConstructor(0, 0);
- return new InlineConstructor();
- }
-}
-
-class InlineConstructorOfInner {
-
- class Inner {
-
- int a;
-
- @CheckDiscarded
- Inner(int a) {
- this.a = a;
- }
-
- // This is not inlined, even though it is only called once, as it is only called from a
- // non-constructor, and will set a field (the outer object) before calling the other
- // constructor.
- Inner(long a) {
- this((int) a);
- }
-
- public Inner create() {
- return new Inner(10L);
- }
- }
-
- Inner inner;
-
- InlineConstructorOfInner() {
- inner = new Inner(10L).create();
- }
-}
-
public class Inlining {
private static void Assert(boolean value) {
@@ -202,6 +92,10 @@
aNumber += longMethodThatWeShouldNotInline("zi", "za", "zo");
aNumber += longMethodThatWeShouldNotInline("do", "de", "da");
System.out.println(aNumber);
+
+ // Call a method that contains a call to a protected method. Should not be inlined.
+ aNumber = new SubClassOfPublicClass().public_protectedMethod(0);
+ System.out.println(aNumber);
}
private static boolean intCmpExpression(A a, A b) {
diff --git a/src/test/examples/inlining/PublicClass.java b/src/test/examples/inlining/PublicClass.java
new file mode 100644
index 0000000..70219ff
--- /dev/null
+++ b/src/test/examples/inlining/PublicClass.java
@@ -0,0 +1,14 @@
+// Copyright (c) 2017, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package inlining;
+
+public class PublicClass {
+
+ protected int protectedMethod(int loopy) {
+ if (loopy > 0) {
+ return protectedMethod(loopy - 1);
+ }
+ return 42;
+ }
+}
diff --git a/src/test/examples/inlining/SubClassOfPublicClass.java b/src/test/examples/inlining/SubClassOfPublicClass.java
new file mode 100644
index 0000000..306ba53
--- /dev/null
+++ b/src/test/examples/inlining/SubClassOfPublicClass.java
@@ -0,0 +1,11 @@
+// Copyright (c) 2017, the R8 project authors. Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+package inlining;
+
+public class SubClassOfPublicClass extends PublicClass {
+
+ public int public_protectedMethod(int loopy) {
+ return protectedMethod(loopy);
+ }
+}
diff --git a/src/test/java/com/android/tools/r8/shaking/TreeShakingTest.java b/src/test/java/com/android/tools/r8/shaking/TreeShakingTest.java
index 8dfcbfe..388e1fe 100644
--- a/src/test/java/com/android/tools/r8/shaking/TreeShakingTest.java
+++ b/src/test/java/com/android/tools/r8/shaking/TreeShakingTest.java
@@ -85,8 +85,7 @@
"examplesAndroidN/shaking:keep-rules.txt:DEX:AGGRESSIVE"
);
- // TODO(65355452): Reenable or remove inlining tests.
- private static Set<String> SKIPPED = ImmutableSet.of("examples/inlining");
+ private static Set<String> SKIPPED = Collections.emptySet();
private final MinifyMode minify;
@@ -657,6 +656,7 @@
"examples/annotationremoval",
"examples/memberrebinding2",
"examples/memberrebinding3",
+ "examples/inlining",
"examples/simpleproto1",
"examples/simpleproto2",
"examples/simpleproto3",
