8140650: Method::is_accessor should cover getters and setters for all types
Reviewed-by: vlivanov, coleenp, sgehwolf
--- a/hotspot/src/cpu/zero/vm/cppInterpreter_zero.cpp Tue Nov 10 21:36:35 2015 -0800
+++ b/hotspot/src/cpu/zero/vm/cppInterpreter_zero.cpp Wed Nov 11 01:27:36 2015 +0300
@@ -497,12 +497,15 @@
// 1: getfield
// 2: index
// 3: index
- // 4: ireturn/areturn
+ // 4: ireturn/areturn/freturn/lreturn/dreturn
// NB this is not raw bytecode: index is in machine order
u1 *code = method->code_base();
assert(code[0] == Bytecodes::_aload_0 &&
code[1] == Bytecodes::_getfield &&
(code[4] == Bytecodes::_ireturn ||
+ code[4] == Bytecodes::_freturn ||
+ code[4] == Bytecodes::_lreturn ||
+ code[4] == Bytecodes::_dreturn ||
code[4] == Bytecodes::_areturn), "should do");
u2 index = Bytes::get_native_u2(&code[2]);
--- a/hotspot/src/share/vm/ci/ciMethod.cpp Tue Nov 10 21:36:35 2015 -0800
+++ b/hotspot/src/share/vm/ci/ciMethod.cpp Wed Nov 11 01:27:36 2015 +0300
@@ -1262,6 +1262,8 @@
bool ciMethod::is_vanilla_constructor() const { FETCH_FLAG_FROM_VM(is_vanilla_constructor); }
bool ciMethod::has_loops () const { FETCH_FLAG_FROM_VM(has_loops); }
bool ciMethod::has_jsrs () const { FETCH_FLAG_FROM_VM(has_jsrs); }
+bool ciMethod::is_getter () const { FETCH_FLAG_FROM_VM(is_getter); }
+bool ciMethod::is_setter () const { FETCH_FLAG_FROM_VM(is_setter); }
bool ciMethod::is_accessor () const { FETCH_FLAG_FROM_VM(is_accessor); }
bool ciMethod::is_initializer () const { FETCH_FLAG_FROM_VM(is_initializer); }
--- a/hotspot/src/share/vm/ci/ciMethod.hpp Tue Nov 10 21:36:35 2015 -0800
+++ b/hotspot/src/share/vm/ci/ciMethod.hpp Wed Nov 11 01:27:36 2015 +0300
@@ -311,6 +311,8 @@
bool is_final_method() const { return is_final() || holder()->is_final(); }
bool has_loops () const;
bool has_jsrs () const;
+ bool is_getter () const;
+ bool is_setter () const;
bool is_accessor () const;
bool is_initializer () const;
bool can_be_statically_bound() const { return _can_be_statically_bound; }
--- a/hotspot/src/share/vm/interpreter/interpreter.cpp Tue Nov 10 21:36:35 2015 -0800
+++ b/hotspot/src/share/vm/interpreter/interpreter.cpp Wed Nov 11 01:27:36 2015 +0300
@@ -300,7 +300,10 @@
}
// Accessor method?
- if (m->is_accessor()) {
+ if (m->is_getter()) {
+ // TODO: We should have used ::is_accessor above, but fast accessors in Zero expect only getters.
+ // See CppInterpreter::accessor_entry in cppInterpreter_zero.cpp. This should be fixed in Zero,
+ // then the call above updated to ::is_accessor
assert(m->size_of_parameters() == 1, "fast code for accessors assumes parameter size = 1");
return accessor;
}
--- a/hotspot/src/share/vm/oops/method.cpp Tue Nov 10 21:36:35 2015 -0800
+++ b/hotspot/src/share/vm/oops/method.cpp Wed Nov 11 01:27:36 2015 +0300
@@ -582,12 +582,45 @@
}
bool Method::is_accessor() const {
+ return is_getter() || is_setter();
+}
+
+bool Method::is_getter() const {
if (code_size() != 5) return false;
if (size_of_parameters() != 1) return false;
- if (java_code_at(0) != Bytecodes::_aload_0 ) return false;
+ if (java_code_at(0) != Bytecodes::_aload_0) return false;
if (java_code_at(1) != Bytecodes::_getfield) return false;
- if (java_code_at(4) != Bytecodes::_areturn &&
- java_code_at(4) != Bytecodes::_ireturn ) return false;
+ switch (java_code_at(4)) {
+ case Bytecodes::_ireturn:
+ case Bytecodes::_lreturn:
+ case Bytecodes::_freturn:
+ case Bytecodes::_dreturn:
+ case Bytecodes::_areturn:
+ break;
+ default:
+ return false;
+ }
+ return true;
+}
+
+bool Method::is_setter() const {
+ if (code_size() != 6) return false;
+ if (java_code_at(0) != Bytecodes::_aload_0) return false;
+ switch (java_code_at(1)) {
+ case Bytecodes::_iload_1:
+ case Bytecodes::_aload_1:
+ case Bytecodes::_fload_1:
+ if (size_of_parameters() != 2) return false;
+ break;
+ case Bytecodes::_dload_1:
+ case Bytecodes::_lload_1:
+ if (size_of_parameters() != 3) return false;
+ break;
+ default:
+ return false;
+ }
+ if (java_code_at(2) != Bytecodes::_putfield) return false;
+ if (java_code_at(5) != Bytecodes::_return) return false;
return true;
}
--- a/hotspot/src/share/vm/oops/method.hpp Tue Nov 10 21:36:35 2015 -0800
+++ b/hotspot/src/share/vm/oops/method.hpp Wed Nov 11 01:27:36 2015 +0300
@@ -595,6 +595,12 @@
// returns true if the method is an accessor function (setter/getter).
bool is_accessor() const;
+ // returns true if the method is a getter
+ bool is_getter() const;
+
+ // returns true if the method is a setter
+ bool is_setter() const;
+
// returns true if the method does nothing but return a constant of primitive type
bool is_constant_getter() const;
--- a/hotspot/src/share/vm/opto/callnode.cpp Tue Nov 10 21:36:35 2015 -0800
+++ b/hotspot/src/share/vm/opto/callnode.cpp Wed Nov 11 01:27:36 2015 +0300
@@ -778,7 +778,7 @@
}
if (is_CallJava() && as_CallJava()->method() != NULL) {
ciMethod* meth = as_CallJava()->method();
- if (meth->is_accessor()) {
+ if (meth->is_getter()) {
return false;
}
// May modify (by reflection) if an boxing object is passed
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/hotspot/test/compiler/inlining/InlineAccessors.java Wed Nov 11 01:27:36 2015 +0300
@@ -0,0 +1,141 @@
+/*
+ * Copyright (c) 2015, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+/**
+ * @test
+ * @bug 8140650
+ * @summary Method::is_accessor should cover getters and setters for all types
+ * @library /testlibrary
+ * @run main/othervm InlineAccessors
+ */
+import java.lang.invoke.*;
+import jdk.test.lib.*;
+import static jdk.test.lib.Asserts.*;
+
+public class InlineAccessors {
+ public static void main(String[] args) throws Exception {
+ // try some sanity checks first
+ doTest();
+
+ ProcessBuilder pb = ProcessTools.createJavaProcessBuilder(
+ "-XX:+IgnoreUnrecognizedVMOptions", "-showversion",
+ "-server", "-XX:-TieredCompilation", "-Xbatch", "-Xcomp",
+ "-XX:+PrintCompilation", "-XX:+UnlockDiagnosticVMOptions", "-XX:+PrintInlining",
+ "InlineAccessors$Launcher");
+
+ OutputAnalyzer analyzer = new OutputAnalyzer(pb.start());
+
+ analyzer.shouldHaveExitValue(0);
+
+ // The test is applicable only to C2 (present in Server VM).
+ if (analyzer.getStderr().contains("Server VM")) {
+ analyzer.shouldContain("InlineAccessors::setBool (6 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::setByte (6 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::setChar (6 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::setShort (6 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::setInt (6 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::setFloat (6 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::setLong (6 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::setDouble (6 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::setObject (6 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::setArray (6 bytes) accessor");
+
+ analyzer.shouldContain("InlineAccessors::getBool (5 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::getByte (5 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::getChar (5 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::getShort (5 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::getInt (5 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::getFloat (5 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::getLong (5 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::getDouble (5 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::getObject (5 bytes) accessor");
+ analyzer.shouldContain("InlineAccessors::getArray (5 bytes) accessor");
+ }
+ }
+
+ boolean bool;
+ byte b;
+ char c;
+ short s;
+ int i;
+ float f;
+ long l;
+ double d;
+ Object o;
+ Object[] a;
+
+ public void setBool(boolean v) { bool = v; }
+ public void setByte(byte v) { b = v; }
+ public void setChar(char v) { c = v; }
+ public void setShort(short v) { s = v; }
+ public void setInt(int v) { i = v; }
+ public void setFloat(float v) { f = v; }
+ public void setLong(long v) { l = v; }
+ public void setDouble(double v) { d = v; }
+ public void setObject(Object v) { o = v; }
+ public void setArray(Object[] v) { a = v; }
+
+ public boolean getBool() { return bool; }
+ public byte getByte() { return b; }
+ public char getChar() { return c; }
+ public short getShort() { return s; }
+ public int getInt() { return i; }
+ public float getFloat() { return f; }
+ public long getLong() { return l; }
+ public double getDouble() { return d; }
+ public Object getObject() { return o; }
+ public Object[] getArray() { return a; }
+
+ static void doTest() {
+ InlineAccessors o = new InlineAccessors();
+ o.setBool(false);
+ o.setByte((byte)0);
+ o.setChar('a');
+ o.setShort((short)0);
+ o.setInt(0);
+ o.setFloat(0F);
+ o.setLong(0L);
+ o.setDouble(0D);
+ o.setObject(new Object());
+ o.setArray(new Object[1]);
+
+ o.getBool();
+ o.getByte();
+ o.getChar();
+ o.getShort();
+ o.getInt();
+ o.getFloat();
+ o.getLong();
+ o.getDouble();
+ o.getObject();
+ o.getArray();
+ }
+
+ static class Launcher {
+ public static void main(String[] args) throws Exception {
+ for (int c = 0; c < 20_000; c++) {
+ doTest();
+ }
+ }
+ }
+}