RFR: 8354543: Set more meaningful names for "get_vm_result" and "get_vm_result_2" [v2]

Aleksey Shipilev shade at openjdk.org
Tue Apr 15 13:44:45 UTC 2025


On Mon, 14 Apr 2025 20:10:33 GMT, Cesar Soares Lucas <cslucas at openjdk.org> wrote:

>> Please, review this trivial PR to set more meaningful names for `get_vm_result*`.
>> 
>> I tested the best I could on OSX AArch64 & Linux amd64 with JTREG tier1-3.
>> The other platforms I tested by cross-compiling. If you can run some tests on those platform I'd appreciate.
>
> Cesar Soares Lucas has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:
> 
>  - Fix merge conflicts
>  - Rename get_vm_result and get_vm_result_2 to more meaningfult names.

I like this. Some stylistic nits:

src/hotspot/cpu/aarch64/templateTable_aarch64.cpp line 3781:

> 3779:   call_VM(r0, CAST_FROM_FN_PTR(address, InterpreterRuntime::quicken_io_cc));
> 3780:   // vm_result_2 has metadata result
> 3781:   __ get_vm_result_metadata(r0, rthread);

Stale comment just above this line. Grep around for `vm_result_2` to look for other cases?

src/hotspot/share/runtime/javaThread.hpp line 149:

> 147:   // Used to pass back results to the interpreter or generated code running Java code.
> 148:   oop           _vm_result_oop;    // oop result is GC-preserved
> 149:   Metadata*     _vm_result_metadata;  // non-oop result

Suggestion:

  oop           _vm_result_oop;      // oop result is GC-preserved
  Metadata*     _vm_result_metadata; // non-oop result

src/hotspot/share/runtime/javaThread.hpp line 788:

> 786:   // Oop results of vm runtime calls
> 787:   oop  vm_result_oop() const                     { return _vm_result_oop; }
> 788:   void set_vm_result_oop(oop x)                  { _vm_result_oop   = x; }

Suggestion:

  void set_vm_result_oop(oop x)                  { _vm_result_oop = x; }

src/hotspot/share/runtime/javaThread.hpp line 790:

> 788:   void set_vm_result_oop(oop x)                  { _vm_result_oop   = x; }
> 789: 
> 790:   void set_vm_result_metadata(Metadata* x)       { _vm_result_metadata   = x; }

Suggestion:

  void set_vm_result_metadata(Metadata* x)       { _vm_result_metadata = x; }

-------------

PR Review: https://git.openjdk.org/jdk/pull/24632#pullrequestreview-2768307025
PR Review Comment: https://git.openjdk.org/jdk/pull/24632#discussion_r2044561913
PR Review Comment: https://git.openjdk.org/jdk/pull/24632#discussion_r2044576179
PR Review Comment: https://git.openjdk.org/jdk/pull/24632#discussion_r2044574537
PR Review Comment: https://git.openjdk.org/jdk/pull/24632#discussion_r2044574898


More information about the hotspot-dev mailing list