RFR: 8307352: AARCH64: Improve itable_stub [v2]

[Volker Simonis]

On Mon, 7 Aug 2023 09:04:57 GMT, Boris Ulasevich <bulasevich at openjdk.org> wrote:

> > I wonder if you can get rid of `temp_reg3` (and the `holder_offset` argument of `MacroAssembler::lookup_interface_method_stub()`) by defining `resolved_klass_reg` as `r17` in `VtableStubs::create_itable_stub()` and defining `Register holder_offset = method_result` as local variable in `MacroAssembler::lookup_interface_method_stub()` (the usages of `holder_offset` and `method_result` don't seem to overlap).
> 
> I think it does not work. Non-overlapping registers are already optimized: resolved_klass and method_result are actually reusing the same rmethod(r12) register.
> 
> > Otherwise looks good.
> 
> Thanks!

Sorry if my comment was unclear. What I meant was this:

diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
index 8bf3b37f471..6e91df7156f 100644
--- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
+++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
@@ -1209,11 +1209,13 @@ void MacroAssembler::lookup_interface_method_stub(Register recv_klass,
                                                   Register method_result,
                                                   Register temp_itbl_klass,
                                                   Register scan_temp,
-                                                  Register holder_offset,
                                                   int itable_index,
                                                   Label& L_no_such_interface) {
+  // 'method_result' is only used as output register at the very end of this method.
+  // Until then we can reuse it as 'holder_offset'.
+  Register holder_offset = method_result;
   assert_different_registers(resolved_klass, recv_klass, holder_klass, temp_itbl_klass, scan_temp, holder_offset);
-  assert_different_registers(method_result, recv_klass, holder_klass, temp_itbl_klass, scan_temp, holder_offset);
+  assert_different_registers(method_result, recv_klass, holder_klass, temp_itbl_klass, scan_temp);
 
   int vtable_start_offset = in_bytes(Klass::vtable_start_offset());
   int itable_offset_entry_size = itableOffsetEntry::size() * wordSize;
diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp
index 72dd98ab617..004bed97b7f 100644
--- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp
+++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.hpp
@@ -949,7 +949,6 @@ class MacroAssembler: public Assembler {
                                     Register method_result,
                                     Register temp_reg,
                                     Register temp_reg2,
-                                    Register temp_reg3,
                                     int itable_index,
                                     Label& L_no_such_interface);
 
diff --git a/src/hotspot/cpu/aarch64/vtableStubs_aarch64.cpp b/src/hotspot/cpu/aarch64/vtableStubs_aarch64.cpp
index 73736566c90..6e345c96ce2 100644
--- a/src/hotspot/cpu/aarch64/vtableStubs_aarch64.cpp
+++ b/src/hotspot/cpu/aarch64/vtableStubs_aarch64.cpp
@@ -175,10 +175,9 @@ VtableStub* VtableStubs::create_itable_stub(int itable_index) {
   // so all registers except arguments are free at this point.
   const Register recv_klass_reg     = r10;
   const Register holder_klass_reg   = r16; // declaring interface klass (DECC)
-  const Register resolved_klass_reg = rmethod; // resolved interface klass (REFC)
+  const Register resolved_klass_reg = r17; // resolved interface klass (REFC)
   const Register temp_reg           = r11;
   const Register temp_reg2          = r15;
-  const Register temp_reg3          = r17;
   const Register icholder_reg       = rscratch2;
 
   Label L_no_such_interface;
@@ -195,7 +194,7 @@ VtableStub* VtableStubs::create_itable_stub(int itable_index) {
   // Receiver subtype check against REFC.
   // Get selected method from declaring class and itable index
   __ lookup_interface_method_stub(recv_klass_reg, holder_klass_reg, resolved_klass_reg, rmethod,
-                                  temp_reg, temp_reg2, temp_reg3, itable_index, L_no_such_interface);
+                                  temp_reg, temp_reg2, itable_index, L_no_such_interface);
 
   // Reduce "estimate" such that "padding" does not drop below 8.
   const ptrdiff_t estimate = 124;

I.e. make `resolved_klass_reg` different from `method_result` such that `method_result` can be reused as `holder_offset` because the two don't overlap. I leave it up to you if you want these changes. A quick check showed that they compile and successfully execute tier1 tests.

Otherwise still looks good.

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

PR Comment: https://git.openjdk.org/jdk/pull/13792#issuecomment-1690505953