United StatesChange Country, Oracle Worldwide Web Sites Communities I am a... I want to...
Bug ID: JDK-6190413 poor constant construction on sparc
JDK-6190413 : poor constant construction on sparc

Details
Type:
Bug
Submit Date:
2004-11-03
Status:
Resolved
Updated Date:
2011-09-15
Project Name:
JDK
Resolved Date:
2005-03-30
Component:
hotspot
OS:
solaris_9
Sub-Component:
compiler
CPU:
sparc
Priority:
P4
Resolution:
Fixed
Affected Versions:
6
Fixed Versions:

Related Reports
Backport:

Sub Tasks

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
EVALUATION

Updated LoadConP to use SetPtr instead of the SetHi/SetLo combo
###@###.### 2005-03-22 17:02:04 GMT
                                     
2005-03-22
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
                                     
2005-03-24



Hardware and Software, Engineered to Work Together