8140650: Method::is_accessor should cover getters and setters for all types

Reviewed-by: vlivanov, coleenp, sgehwolf

hotspot/src/cpu/zero/vm/cppInterpreter_zero.cpp

@@ -497,12 +497,15 @@ int CppInterpreter::accessor_entry(Method* method, intptr_t UNUSED, TRAPS) {
   //  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]);
 

hotspot/src/share/vm/ci/ciMethod.cpp

@@ -1262,6 +1262,8 @@ bool ciMethod::is_empty_method() const {         FETCH_FLAG_FROM_VM(is_empty_met
 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); }
 

hotspot/src/share/vm/ci/ciMethod.hpp

@@ -311,6 +311,8 @@ class ciMethod : public ciMetadata {
   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; }

hotspot/src/share/vm/interpreter/interpreter.cpp

@@ -300,7 +300,10 @@ AbstractInterpreter::MethodKind AbstractInterpreter::method_kind(methodHandle m)
   }
 
   // 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;
   }

hotspot/src/share/vm/oops/method.cpp

@@ -582,12 +582,45 @@ bool Method::can_be_statically_bound() const {
 }
 
 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 &&
+  switch (java_code_at(4)) {
-      java_code_at(4) != Bytecodes::_ireturn ) return false;
+    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;
 }
 

hotspot/src/share/vm/oops/method.hpp

@@ -595,6 +595,12 @@ class Method : public Metadata {
   // 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;
 

hotspot/src/share/vm/opto/callnode.cpp

@@ -778,7 +778,7 @@ bool CallNode::may_modify(const TypeOopPtr *t_oop, PhaseTransform *phase) {
     }
     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

hotspot/test/compiler/inlining/InlineAccessors.java

@@ -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();
+            }
+        }
+    }
+}