JDK-6190413 : poor constant construction on sparc
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 6
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • OS: solaris_9
  • CPU: sparc
  • Submitted: 2004-11-03
  • Updated: 2011-09-15
  • Resolved: 2005-03-30
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other JDK 6
5.0u5Fixed 6 b30Fixed
Description
While look at a some assembly on sparc I noticed that C2 doesn't sometimes generate an extra useless instruction to produce constants of pointer type.  In particular the effects the generation the initial mark word 0x1 and the card mark byte_map_base values.  For example:

  0xf90b53c4: sethi  %hi(0), %i1
  0xf90b53c8: or  %i1, 1, %i1   ! 0x00000001
  0xf90b53cc: sethi  %hi(0xfd62a000), %i0
  0xf90b53d0: mov  %i0, %i0     ! word_map_base

64-bit seems to handle this slightly better for some reason.  I think the problem is in sparc.ad in loadConP which forces the emission of the sethi/add pair for all pointers unlike the 64-bit version which checks for an oop first.  Look at SetPtr64.  This could probably be unified and remove the LP64 test and would end up producing slightly better code.  Here's the trivial program I used to get that assembly.

public class p {
    public static void main(String[] args) {
        while (true) {
            for (int i = 0; i < args.length; i++) {
                args[i] = new String(args[i]);
            }
        }
    }
}

Often these values get commoned up as they do in this case but not always so it's worth correcting.
###@###.### 11/3/04 22:48 GMT

Comments
SUGGESTED FIX --- sparc.ad Thu Mar 24 10:39:09 2005 *** 1,11 **** --- 1,11 ---- - // @(#)sparc.ad 1.414 05/03/17 12:35:48 + // @(#)sparc.ad 1.415 05/03/24 10:39:06 // // Copyright 2004 Sun Microsystems, Inc. All rights reserved. // SUN PROPRIETARY/CONFIDENTIAL. Use is subject to license terms. // // SPARC Architecture Description File //----------REGISTER DEFINITION BLOCK------------------------------------------ // This information is used by the matcher and the register allocator to // describe individual registers and classes of registers within the target *** 434,454 **** --- 434,454 ---- // Macros to extract hi & lo halves from a long pair. // G0 is not part of any long pair, so assert on that. // Prevents accidently using G1 instead of G0. #define LONG_HI_REG(x) (x) #define LONG_LO_REG(x) (x) %} source %{ - #pragma ident "@(#)sparc.ad 1.414 05/03/17 12:35:48 JVM" + #pragma ident "@(#)sparc.ad 1.415 05/03/24 10:39:06 JVM" #define __ _masm. // tertiary op of a LoadP or StoreP encoding #define REGP_OP true static FloatRegister reg_to_SingleFloatRegister_object(int register_encoding); static FloatRegister reg_to_DoubleFloatRegister_object(int register_encoding); static Register reg_to_register_object(int register_encoding); *** 2358,2422 **** --- 2358,2391 ---- (Assembler::movcc_op3 << 19) | (6 << 16) | // cc2 bit for 'xcc' ($primary << 14) | (0 << 13) | // select register move (0 << 11) | // cc1, cc0 bits for 'icc' ($src$$reg << 0); *((int*)(cbuf.code_end())) = op; cbuf.set_code_end(cbuf.code_end() + BytesPerInstWord); %} #ifdef _LP64 // Utility encoding for loading a 64 bit Pointer into a register // The 64 bit pointer is stored in the generated code stream ! enc_class SetPtr64( immP src, iRegP rd ) %{ Register dest = reg_to_register_object($rd$$reg); // [RGV] This next line should be generated from ADLC if ( _opnds[1]->constant_is_oop() ) { add_oop_Relocation(cbuf, (jobject) $src$$constant, 0); emit_ptr(cbuf, $src$$constant, dest, /*ForceRelocatable=*/ true); + intptr_t val = $src$$constant; + MacroAssembler _masm(&cbuf); + __ set_oop((jobject)val, dest); } else { // non-oop pointers, e.g. card mark base, heap top emit_ptr(cbuf, $src$$constant, dest, /*ForceRelocatable=*/ false); } %} #endif // _LP64 // Utility encodings for loading an immediate value into a register enc_class SetHi( immP src, iRegP rd ) %{ cbuf.set_mark(); $$$emit_hi$src$$constant; // Relocation bits emit2_22( cbuf, Assembler::branch_op, $rd$$reg, Assembler::sethi_op2, $src$$constant ); %} enc_class SetLo_or_nop_if_null( immP src, iRegP rd ) %{ if ($src$$constant != 0) { cbuf.set_mark(); $$$emit_lo$src$$constant; // Relocation bits emit3_simm10( cbuf, Assembler::arith_op, $rd$$reg, Assembler::or_op3, $rd$$reg, $src$$constant ); #ifdef ASSERT if (VerifyOops) { const TypePtr* tp = this->bottom_type()->is_ptr(); if (tp->isa_oop_ptr()) { MacroAssembler _masm(&cbuf); int offset = tp->offset(); assert(offset != Type::OffsetBot, "must be constant offset"); Register x = reg_to_register_object($rd$$reg); if (offset) __ untested("verifying oop+offset constant"); if (offset) __ dec(x, offset); __ verify_oop(x); if (offset) __ inc(x, offset); } } #endif } %} enc_class Set13( immI13 src, iRegI rd ) %{ emit3_simm13( cbuf, Assembler::arith_op, $rd$$reg, Assembler::or_op3, 0, $src$$constant ); %} enc_class SetHi22( immI src, iRegI rd ) %{ emit2_22( cbuf, Assembler::branch_op, $rd$$reg, Assembler::sethi_op2, $src$$constant ); %} enc_class Set32( immI src, iRegI rd ) %{ MacroAssembler _masm(&cbuf); *** 2674,2715 **** --- 2643,2680 ---- Register Roop = reg_to_register_object($oop$$reg); Register Rbox = reg_to_register_object($box$$reg); Register Rscratch = G3; Register Rmark = G4; assert(Roop != Rscratch, ""); assert(Roop != Rmark, ""); assert(Rbox != Rscratch, ""); assert(Rbox != Rmark, ""); Address mark_addr(Roop, 0, oopDesc::mark_offset_in_bytes()); __ compiler_lock_object(Roop, Rmark, Rbox, Rscratch, done); __ bind(done); %} enc_class Fast_Unlock (iRegP oop, iRegP box) %{ MacroAssembler _masm(&cbuf); Label done; Register Roop = reg_to_register_object($oop$$reg); Register Rbox = reg_to_register_object($box$$reg); Register Rscratch = G3; Register Rmark = G4; assert(Roop != Rscratch, ""); assert(Roop != Rmark, ""); assert(Rbox != Rscratch, ""); assert(Rbox != Rmark, ""); Address mark_addr(Roop, 0, oopDesc::mark_offset_in_bytes()); __ compiler_unlock_object(Roop, Rmark, Rbox, Rscratch, done); // Done. Adios __ bind(done); %} enc_class enc_cas( iRegP mem, iRegP old, iRegP new ) %{ MacroAssembler _masm(&cbuf); Register Rmem = reg_to_register_object($mem$$reg); Register Rold = reg_to_register_object($old$$reg); *** 4317,4343 **** --- 4282,4306 ---- %} // Long Constant pipe_class loadConL( iRegL dst, immL src ) %{ instruction_count(2); multiple_bundles; dst : E(write)+1; IALU : R(2); IALU : R(2); %} #ifdef _LP64 // Pointer Constant pipe_class loadConP( iRegP dst, immP src ) %{ instruction_count(0); multiple_bundles; fixed_latency(6); %} #endif // Polling Address pipe_class loadConP_poll( iRegP dst, immP_poll src ) %{ #ifdef _LP64 instruction_count(0); multiple_bundles; fixed_latency(6); #else dst : E(write); IALU : R; #endif *** 5372,5398 **** --- 5335,5357 ---- %} instruct loadConP(iRegP dst, immP src) %{ match(Set dst src); ins_cost(DEFAULT_COST * 3/2); format %{ "SET $src,$dst\t!ptr" %} // This rule does not use "expand" unlike loadConI because then // the result type is not known to be an Oop. An ADLC // enhancement will be needed to make that work - not worth it! - #ifndef _LP64 ins_encode( SetHi( src, dst ), SetLo_or_nop_if_null( src, dst ) ); ins_pipe(ialu_hi_lo_reg); #else ins_encode( SetPtr64( src, dst ) ); + ins_encode( SetPtr( src, dst ) ); ins_pipe(loadConP); #endif + %} instruct loadConP0(iRegP dst, immP0 src) %{ match(Set dst src); size(4); format %{ "CLR $dst\t!ptr" %} ins_encode( SetNull( dst ) ); ins_pipe(ialu_imm); %} ###@###.### 2005-03-24 18:51:27 GMT
24-03-2005

EVALUATION Updated LoadConP to use SetPtr instead of the SetHi/SetLo combo ###@###.### 2005-03-22 17:02:04 GMT
22-03-2005