Request for review(XL): 6985015: C1 needs to support compressed oops

[Christian Thalinger]

On Nov 19, 2010, at 4:12 AM, Igor Veresov wrote:
> This change implements compressed oops for C1 for x64 and sparc.
> The changes are mostly on the codegen level, with a few exceptions  
> when we do access things outside of the heap that are uncompressed  
> from the IR.
>
> Webrev: http://cr.openjdk.java.net/~iveresov/6985015/webrev.00/

src/cpu/sparc/vm/c1_LIRAssembler_sparc.cpp:

  764 #ifdef _LP64
  765   if (UseCompressedOops) {
  766     __ lduw(O0, oopDesc::klass_offset_in_bytes(), G3_scratch);
  767     __ decode_heap_oop(G3_scratch);
  768   } else
  769 #endif
  770     __ ld_ptr(O0, oopDesc::klass_offset_in_bytes(), G3_scratch);
Why not using load_heap_oop here?  I wonder why there is no  
load_klass_null that does all that (or actually the current load_klass  
should be called load_klass_not_null, but that's a too big change).

  949             __ decode_heap_oop(to_reg->as_register(), to_reg- 
 >as_register());
  993             __ decode_heap_oop(to_reg->as_register(), to_reg- 
 >as_register());
Pass the register only once, it's a little shorter.

Maybe you should add a local method doing:

  946 #ifdef _LP64
  947           if (UseCompressedOops && !wide) {
  948             __ lduw(base, offset, to_reg->as_register());
  949             __ decode_heap_oop(to_reg->as_register(), to_reg- 
 >as_register());
  950           } else
  951 #endif
  952             __ ld_ptr(base, offset, to_reg->as_register());
and reuse it.

1240 //          load(O7, 0, to_reg, T_INT, false /*wide*/, false / 
*unaligned*/);
Why is it commented?

src/cpu/x86/vm/c1_LIRAssembler_x86.cpp:

  550   __ load_heap_oop(rsi, Address(rax,  
java_lang_String::value_offset_in_bytes()));
  551   __ movptr (rcx, Address(rax,  
java_lang_String::offset_offset_in_bytes()));
  552   __ lea    (rsi, Address(rsi, rcx, Address::times_2,  
arrayOopDesc::base_offset_in_bytes(T_CHAR)));
  557   __ load_heap_oop(rdi, Address(rbx,  
java_lang_String::value_offset_in_bytes()));
  558   __ movptr (rcx, Address(rbx,  
java_lang_String::offset_offset_in_bytes()));
  559   __ lea    (rdi, Address(rdi, rcx, Address::times_2,  
arrayOopDesc::base_offset_in_bytes(T_CHAR)));
Please keep the indent.

  779     case T_ADDRESS:
  780     __ movptr(frame_map()->address_for_slot(dest- 
 >single_stack_ix()), c->as_jint_bits());
  781       break;
Indent.

src/cpu/x86/vm/c1_LIRGenerator_x86.cpp:

1143   if (!x->klass()->is_loaded() || LP64_ONLY(UseCompressedOops)  
NOT_LP64(false)) {
1165   if (!x->klass()->is_loaded() || LP64_ONLY(UseCompressedOops)  
NOT_LP64(false)) {
I think the common pattern is LP64_ONLY(|| UseCompressedOops).

src/share/vm/c1/c1_LIRAssembler.hpp:

    void mem2reg    (LIR_Opr src, LIR_Opr dest, BasicType type,
-                   LIR_PatchCode patch_code = lir_patch_none,
-                   CodeEmitInfo* info = NULL, bool unaligned = false);
+                   LIR_PatchCode patch_code,
+                   CodeEmitInfo* info, bool unaligned, bool wide);
Why did you remove the default values?  Because wide is always passed  
in?

Otherwise looks good.

-- Christian