RFR (S) 8140650: Method::is_accessor should cover getters and setters for all types

[Vladimir Ivanov]

Overall, looks good.

How much work is it required on Zero side to make it work? It would be 
good to avoid shared code pollution (even temporal) just for Zero 
purposes. My concern is that it will stay that way forever.

If it is too much work for you, I'd ask Zero maintainers for help and 
coordinate the changes.

Best regards,
Vladimir Ivanov

On 11/3/15 6:20 PM, Aleksey Shipilev wrote:
> Hi,
>
> I would like to have a formal review for a minor nit in
> Method::is_accessor. The definition for this method is inconsistent with
> its intent: it should accept all accessors, but instead it only  accepts
> the specific shapes of getters, and completely ignores setters. See:
>     https://bugs.openjdk.java.net/browse/JDK-8140650
>
> This makes compilers to ignore many trivial methods that we might
> otherwise inline when all other inline hints have failed. It seems to be
> a lingering issue left from interpreters that had the "fast accessors".
> While it is an open question should inlining policy treat accessors
> differently or not (I stand by "yes, it should"), this is a fix that
> makes is_accessors proper:
>     http://cr.openjdk.java.net/~shade/8140650/webrev.01/
>
> The only usage for the "old" style is_accessor is Zero, and they would
> like to update them after we commit the change:
>     http://mail.openjdk.java.net/pipermail/zero-dev/2015-November/000551.html
>
> The patch passes JPRT, RBT (hotspot_all), and the new regression test.
>
> It beats me whether this is a runtime, or compiler change -- JIRA bug
> flip-flops on that -- so, sending to hotspot-dev at . I think it can be
> pushed via hs-comp, given it impacts compilers mostly, and has the
> compiler-specific test.
>
> Thanks,
> -Aleksey
>
>