RFR: 8331861: [PPC64] Implement load / store assembler functions which take an Address object
Amit Kumar
amitkumar at openjdk.org
Mon Oct 28 16:40:19 UTC 2024
On Mon, 14 Oct 2024 12:20:45 GMT, Varada M <varadam at openjdk.org> wrote:
> Load and store assembly instructions which takes Address object as argument.
>
> Tier 1 testing successful on linux-ppc64le and aix-ppc (fastdebug level)
>
> JBS : [JDK-8331861](https://bugs.openjdk.org/browse/JDK-8331861)
Can you apply this patch, I did the change with regex, so Please verify before pushing it.
diff --git a/src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp b/src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp
index 684c06614a9..f9207d89615 100644
--- a/src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp
+++ b/src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp
@@ -157,9 +157,9 @@ void LIR_Assembler::osr_entry() {
mo = frame_map()->address_for_monitor_object(i);
assert(ml.index() == noreg && mo.index() == noreg, "sanity");
__ ld(R0, slot_offset + 0, OSR_buf);
- __ std(R0, ml.disp(), ml.base());
+ __ std(R0, ml);
__ ld(R0, slot_offset + 1*BytesPerWord, OSR_buf);
- __ std(R0, mo.disp(), mo.base());
+ __ std(R0, mo);
}
}
}
@@ -581,14 +581,14 @@ void LIR_Assembler::emit_opConvert(LIR_OpConvert* op) {
__ fcmpu(CCR0, rsrc, rsrc);
if (dst_in_memory) {
__ li(R0, 0); // 0 in case of NAN
- __ std(R0, addr.disp(), addr.base());
+ __ std(R0, addr);
} else {
__ li(dst->as_register(), 0);
}
__ bso(CCR0, L);
__ fctiwz(rsrc, rsrc); // USE_KILL
if (dst_in_memory) {
- __ stfd(rsrc, addr.disp(), addr.base());
+ __ stfd(rsrc, addr);
} else {
__ mffprd(dst->as_register(), rsrc);
}
@@ -605,14 +605,14 @@ void LIR_Assembler::emit_opConvert(LIR_OpConvert* op) {
__ fcmpu(CCR0, rsrc, rsrc);
if (dst_in_memory) {
__ li(R0, 0); // 0 in case of NAN
- __ std(R0, addr.disp(), addr.base());
+ __ std(R0, addr);
} else {
__ li(dst->as_register_lo(), 0);
}
__ bso(CCR0, L);
__ fctidz(rsrc, rsrc); // USE_KILL
if (dst_in_memory) {
- __ stfd(rsrc, addr.disp(), addr.base());
+ __ stfd(rsrc, addr);
} else {
__ mffprd(dst->as_register_lo(), rsrc);
}
@@ -873,20 +873,20 @@ void LIR_Assembler::const2stack(LIR_Opr src, LIR_Opr dest) {
int value = c->as_jint_bits();
__ load_const_optimized(src_reg, value);
Address addr = frame_map()->address_for_slot(dest->single_stack_ix());
- __ stw(src_reg, addr.disp(), addr.base());
+ __ stw(src_reg, addr);
break;
}
case T_ADDRESS: {
int value = c->as_jint_bits();
__ load_const_optimized(src_reg, value);
Address addr = frame_map()->address_for_slot(dest->single_stack_ix());
- __ std(src_reg, addr.disp(), addr.base());
+ __ std(src_reg, addr);
break;
}
case T_OBJECT: {
jobject2reg(c->as_jobject(), src_reg);
Address addr = frame_map()->address_for_slot(dest->single_stack_ix());
- __ std(src_reg, addr.disp(), addr.base());
+ __ std(src_reg, addr);
break;
}
case T_LONG:
@@ -894,7 +894,7 @@ void LIR_Assembler::const2stack(LIR_Opr src, LIR_Opr dest) {
int value = c->as_jlong_bits();
__ load_const_optimized(src_reg, value);
Address addr = frame_map()->address_for_double_slot(dest->double_stack_ix());
- __ std(src_reg, addr.disp(), addr.base());
+ __ std(src_reg, addr);
break;
}
default:
@@ -1070,24 +1070,24 @@ void LIR_Assembler::stack2stack(LIR_Opr src, LIR_Opr dest, BasicType type) {
case T_FLOAT: {
Address from = frame_map()->address_for_slot(src->single_stack_ix());
Address to = frame_map()->address_for_slot(dest->single_stack_ix());
- __ lwz(tmp, from.disp(), from.base());
- __ stw(tmp, to.disp(), to.base());
+ __ lwz(tmp, from);
+ __ stw(tmp, to);
break;
}
case T_ADDRESS:
case T_OBJECT: {
Address from = frame_map()->address_for_slot(src->single_stack_ix());
Address to = frame_map()->address_for_slot(dest->single_stack_ix());
- __ ld(tmp, from.disp(), from.base());
- __ std(tmp, to.disp(), to.base());
+ __ ld(tmp, from);
+ __ std(tmp, to);
break;
}
case T_LONG:
case T_DOUBLE: {
Address from = frame_map()->address_for_double_slot(src->double_stack_ix());
Address to = frame_map()->address_for_double_slot(dest->double_stack_ix());
- __ ld(tmp, from.disp(), from.base());
- __ std(tmp, to.disp(), to.base());
+ __ ld(tmp, from);
+ __ std(tmp, to);
break;
}
diff --git a/src/hotspot/cpu/ppc/gc/z/zBarrierSetAssembler_ppc.cpp b/src/hotspot/cpu/ppc/gc/z/zBarrierSetAssembler_ppc.cpp
index 89ab1b1edee..0307c60087f 100644
--- a/src/hotspot/cpu/ppc/gc/z/zBarrierSetAssembler_ppc.cpp
+++ b/src/hotspot/cpu/ppc/gc/z/zBarrierSetAssembler_ppc.cpp
@@ -610,14 +610,14 @@ void ZBarrierSetAssembler::try_resolve_jobject_in_native(MacroAssembler* masm, R
// Resolve global handle
__ ld(dst, 0, dst);
- __ ld(tmp, load_bad_mask.disp(), load_bad_mask.base());
+ __ ld(tmp, load_bad_mask);
__ b(check_color);
__ bind(weak_tagged);
// Resolve weak handle
__ ld(dst, 0, dst);
- __ ld(tmp, mark_bad_mask.disp(), mark_bad_mask.base());
+ __ ld(tmp, mark_bad_mask);
__ bind(check_color);
__ and_(tmp, tmp, dst);
Looks Good
src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp line 160:
> 158: assert(ml.index() == noreg && mo.index() == noreg, "sanity");
> 159: __ ld(R0, slot_offset + 0, OSR_buf);
> 160: __ std(R0, ml, noreg);
No, `tmp=noreg` is default case.
So
Suggestion:
__ std(R0, ml);
should work.
src/hotspot/cpu/ppc/c1_LIRAssembler_ppc.cpp line 1090:
> 1088: Address to = frame_map()->address_for_double_slot(dest->double_stack_ix());
> 1089: __ ld(tmp, from);
> 1090: __ std(tmp, to.disp(), to.base());
why these store instruction left behind ?
Can't we just pass address and give it scratch register to play with ?
-------------
Changes requested by amitkumar (Committer).
PR Review: https://git.openjdk.org/jdk/pull/21492#pullrequestreview-2366620862
Marked as reviewed by amitkumar (Committer).
PR Review: https://git.openjdk.org/jdk/pull/21492#pullrequestreview-2394406675
PR Review Comment: https://git.openjdk.org/jdk/pull/21492#discussion_r1816086379
PR Review Comment: https://git.openjdk.org/jdk/pull/21492#discussion_r1814744899
More information about the hotspot-compiler-dev
mailing list