assert_different_regs is a mechanism we use to ensure that we don't use the same register in different variables. When the assert triggers it is not immediately clear where and why the assert failed.
For example, if I introduce an intentional violation:
diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
index fde868a64b3..551878ac09d 100644
--- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
+++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
@@ -1188,7 +1188,8 @@ void MacroAssembler::lookup_interface_method(Register recv_klass,
Register scan_temp,
Label& L_no_such_interface,
bool return_method) {
- assert_different_registers(recv_klass, intf_klass, scan_temp);
+ Register joker = intf_klass;
+ assert_different_registers(recv_klass, intf_klass, scan_temp, joker);
assert_different_registers(method_result, intf_klass, scan_temp);
assert(recv_klass != method_result || !return_method,
"recv_klass can be destroyed when method isn't needed");
I get this error message:
# Internal Error (src/hotspot/share/asm/register.hpp:287), pid=42568, tid=9731
# assert(!regs[i]->is_valid() || regs[i] != regs[j]) failed: Multiple uses of register: c_rarg0
The indicated file and line number refers to the assert_different_registers implementation and not the offending call site. More over, it's unclear from the assert which of the four variables contain the same register.
I'd like to propose a few changes:
1) That we report the indices of the conflicting registers
2) That we report the correct file and line number
3) That we hide the is_valid() check to lower the noise in the output. Not strictly necessary, but I think it looks nicer.
After these suggestions we'll get error messages that look like this:
# Internal Error (src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp:1187), pid=59065, tid=8963
# assert(regs[i] != regs[j]) failed: regs[1] and regs[3] are both: c_rarg0
Which makes it easy to see that variables 1 and 3 are conflicting and by looking at the indicated file and line, it is clear that it is intf_klass and joker that are the offending variables.
There might be a way to use more macros to propagate the variable names, but I propose that we start with this incremental improvement.
For example, if I introduce an intentional violation:
diff --git a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
index fde868a64b3..551878ac09d 100644
--- a/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
+++ b/src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp
@@ -1188,7 +1188,8 @@ void MacroAssembler::lookup_interface_method(Register recv_klass,
Register scan_temp,
Label& L_no_such_interface,
bool return_method) {
- assert_different_registers(recv_klass, intf_klass, scan_temp);
+ Register joker = intf_klass;
+ assert_different_registers(recv_klass, intf_klass, scan_temp, joker);
assert_different_registers(method_result, intf_klass, scan_temp);
assert(recv_klass != method_result || !return_method,
"recv_klass can be destroyed when method isn't needed");
I get this error message:
# Internal Error (src/hotspot/share/asm/register.hpp:287), pid=42568, tid=9731
# assert(!regs[i]->is_valid() || regs[i] != regs[j]) failed: Multiple uses of register: c_rarg0
The indicated file and line number refers to the assert_different_registers implementation and not the offending call site. More over, it's unclear from the assert which of the four variables contain the same register.
I'd like to propose a few changes:
1) That we report the indices of the conflicting registers
2) That we report the correct file and line number
3) That we hide the is_valid() check to lower the noise in the output. Not strictly necessary, but I think it looks nicer.
After these suggestions we'll get error messages that look like this:
# Internal Error (src/hotspot/cpu/aarch64/macroAssembler_aarch64.cpp:1187), pid=59065, tid=8963
# assert(regs[i] != regs[j]) failed: regs[1] and regs[3] are both: c_rarg0
Which makes it easy to see that variables 1 and 3 are conflicting and by looking at the indicated file and line, it is clear that it is intf_klass and joker that are the offending variables.
There might be a way to use more macros to propagate the variable names, but I propose that we start with this incremental improvement.
- links to
-
Commit(master) openjdk/jdk/c6721a0f
-
Review(master) openjdk/jdk/20965