# HG changeset patch # User shade # Date 1447194456 -10800 # Node ID b0b7187852b7b3561e42d78af5838250e82f915e # Parent dbdc6907b55daf42218a42697818f7e9a5efa025 8140650: Method::is_accessor should cover getters and setters for all types Reviewed-by: vlivanov, coleenp, sgehwolf diff -r dbdc6907b55d -r b0b7187852b7 hotspot/src/cpu/zero/vm/cppInterpreter_zero.cpp --- 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]); diff -r dbdc6907b55d -r b0b7187852b7 hotspot/src/share/vm/ci/ciMethod.cpp --- 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); } diff -r dbdc6907b55d -r b0b7187852b7 hotspot/src/share/vm/ci/ciMethod.hpp --- 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; } diff -r dbdc6907b55d -r b0b7187852b7 hotspot/src/share/vm/interpreter/interpreter.cpp --- 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; } diff -r dbdc6907b55d -r b0b7187852b7 hotspot/src/share/vm/oops/method.cpp --- 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; } diff -r dbdc6907b55d -r b0b7187852b7 hotspot/src/share/vm/oops/method.hpp --- 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; diff -r dbdc6907b55d -r b0b7187852b7 hotspot/src/share/vm/opto/callnode.cpp --- 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 diff -r dbdc6907b55d -r b0b7187852b7 hotspot/test/compiler/inlining/InlineAccessors.java --- /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(); + } + } + } +}