From 85650c0dee5967b6346f16c751e3f4f58c239fa1 Mon Sep 17 00:00:00 2001 From: Andrew Haley Date: Mon, 3 Aug 2026 14:32:21 +0100 Subject: [PATCH] Review comments --- .../cpu/aarch64/c1_LIRAssembler_aarch64.cpp | 19 ++++------ src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp | 36 ++++++++----------- src/hotspot/share/c1/c1_LIRGenerator.cpp | 2 +- 3 files changed, 21 insertions(+), 36 deletions(-) diff --git a/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp b/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp index a40eee0aaff..73b4edc616c 100644 --- a/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp +++ b/src/hotspot/cpu/aarch64/c1_LIRAssembler_aarch64.cpp @@ -2548,18 +2548,17 @@ void LIR_Assembler::increment_profile_ctr(LIR_Opr step, LIR_Opr dest_opr, LIR_Op CodeStub* overflow_stub) { #ifndef PRODUCT if (CommentedAssembly) { - __ block_comment("increment_event_counter {"); + __ block_comment("increment_profile_ctr" " {"); } #endif - int profile_capture_ratio = ProfileCaptureRatio; - int ratio_shift = exact_log2(profile_capture_ratio); + int ratio_shift = exact_log2(ProfileCaptureRatio); uint64_t threshold = (UCONST64(1) << 32) >> ratio_shift; assert(threshold > 0, "must be"); ProfileStub *counter_stub - = profile_capture_ratio > 1 ? new ProfileStub() : nullptr; + = ProfileCaptureRatio > 1 ? new ProfileStub() : nullptr; Register dest = as_reg(dest_opr); @@ -2589,7 +2588,6 @@ void LIR_Assembler::increment_profile_ctr(LIR_Opr step, LIR_Opr dest_opr, LIR_Op if (ProfileCaptureRatio > 1) { __ lsl(inc, inc, ratio_shift); } - __ ldrw(dest, counter_address); __ addw(dest, dest, inc); __ strw(dest, counter_address); @@ -2597,13 +2595,8 @@ void LIR_Assembler::increment_profile_ctr(LIR_Opr step, LIR_Opr dest_opr, LIR_Op __ lsr(inc, inc, ratio_shift); } } else { - jint inc = step->as_constant_ptr()->as_jint_bits(); - inc *= ProfileCaptureRatio; + jint inc = step->as_constant_ptr()->as_jint_bits() * ProfileCaptureRatio; switch (dest_opr->type()) { - case T_INT: { - __ incrementw(counter_address, inc, dest); - break; - } case T_LONG: { __ increment(counter_address, inc, dest); break; @@ -2668,7 +2661,7 @@ void LIR_Assembler::increment_profile_ctr(LIR_Opr step, LIR_Opr dest_opr, LIR_Op __ step_random(r_profile_rng, rscratch2); counter_stub->set_action(lambda, nullptr); - counter_stub->set_name("IncrementEventCounter"); + counter_stub->set_name("IncrementProfileCtr"); append_code_stub(counter_stub); } else { lambda(this, nullptr); @@ -2676,7 +2669,7 @@ void LIR_Assembler::increment_profile_ctr(LIR_Opr step, LIR_Opr dest_opr, LIR_Op #ifndef PRODUCT if (CommentedAssembly) { - __ block_comment("} increment_event_counter"); + __ block_comment("} increment_profile_ctr"); } #endif } diff --git a/src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp b/src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp index 139c3cea21f..23859a1c6d2 100644 --- a/src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp +++ b/src/hotspot/cpu/x86/c1_LIRAssembler_x86.cpp @@ -2807,25 +2807,24 @@ void LIR_Assembler::increment_profile_ctr(LIR_Opr step_opr, LIR_Opr dest_opr, auto lambda = [counter_stub, overflow_stub, freq_opr, ratio_shift, step_opr, md_reg, md_opr, md_offset_opr, dest_opr, dest] (LIR_Assembler* ce, LIR_Op* op) { - auto masm = [=]() { return ce->masm(); }; - Address counter_address; + auto masm = [ce]() { return ce->masm(); }; if (counter_stub != nullptr) __ bind(*counter_stub->entry()); - if (md_opr->is_valid()) { - if (md_opr->type() == T_METADATA) { - __ mov_metadata(md_reg->as_register(), - md_opr->as_constant_ptr()->as_metadata()); - } else { - __ lea(md_reg->as_pointer_register(), - ExternalAddress(md_opr->as_constant_ptr()->as_pointer())); - } - RegisterOrConstant offset = - md_offset_opr->is_constant() + assert(md_opr->is_valid(), "must be"); + + if (md_opr->type() == T_METADATA) { + __ mov_metadata(md_reg->as_register(), + md_opr->as_constant_ptr()->as_metadata()); + } else { + __ lea(md_reg->as_pointer_register(), + ExternalAddress(md_opr->as_constant_ptr()->as_pointer())); + } + RegisterOrConstant offset = + md_offset_opr->is_constant() ? RegisterOrConstant(md_offset_opr->as_constant_ptr()->as_jint()) : as_reg(md_offset_opr); - counter_address = Address(md_reg->as_pointer_register(), offset); - } + counter_address = Address(md_reg->as_pointer_register(), offset); if (step_opr->is_register()) { Register inc = step_opr->as_register(); @@ -2841,13 +2840,6 @@ void LIR_Assembler::increment_profile_ctr(LIR_Opr step_opr, LIR_Opr dest_opr, } else { jint inc = step_opr->as_constant_ptr()->as_jint_bits() * ProfileCaptureRatio; switch (dest_opr->type()) { - case T_INT: { - __ movl(dest, counter_address); - // Use lea instead of add to avoid destroying condition codes on x86 - __ lea(dest, Address(dest, inc, Address::times_1)); - __ movl(counter_address, dest); - break; - } case T_LONG: { __ movq(dest, counter_address); // Use lea instead of add to avoid destroying condition codes on x86 @@ -2907,7 +2899,7 @@ void LIR_Assembler::increment_profile_ctr(LIR_Opr step_opr, LIR_Opr dest_opr, }; if (counter_stub != nullptr) { - __ cmpl(r_profile_rng, threshold); + __ cmpl(r_profile_rng, checked_cast(threshold)); __ jcc(Assembler::below, *counter_stub->entry()); __ bind(*counter_stub->continuation()); __ step_random(r_profile_rng, dest); diff --git a/src/hotspot/share/c1/c1_LIRGenerator.cpp b/src/hotspot/share/c1/c1_LIRGenerator.cpp index 0fb30c8e613..3addd3be016 100644 --- a/src/hotspot/share/c1/c1_LIRGenerator.cpp +++ b/src/hotspot/share/c1/c1_LIRGenerator.cpp @@ -964,7 +964,7 @@ void LIRGenerator::profile_branch(If* if_instr, If::Condition cond) { LIR_OprFact::intptrConst(not_taken_count_offset), data_offset_reg, as_BasicType(if_instr->x()->type())); - LIR_Opr tmp = new_register(wordSize == 4 ? T_INT : T_LONG); + LIR_Opr tmp = new_pointer_register(); // really an intptr_t counter LIR_Opr step = LIR_OprFact::intConst(DataLayout::counter_increment); __ increment_counter(step, tmp, md_reg, md->constant_encoding(), data_offset_reg); }