RFR: Backport SFX-related changes to sh/jdk8

[Aleksey Shipilev]

On 5/23/20 10:40 PM, Roman Kennke wrote:
> http://cr.openjdk.java.net/~rkennke/sfx-jdk8/webrev.03/
Cannot shake the feeling this is too risky for July release.

*) We cannot pick up changesets like that:
  8202976: Add C1 lea patching support for x86

...because it messes up tracking. It should be something like:
  Cherry-pick 8202976: Add C1 lea patching support for x86

*) src/cpu/aarch64/vm/c1_LIRAssembler_aarch64.cpp: the new block should be guarded by UseShenandoahGC?

*) src/cpu/x86/vm/c1_LIRAssembler_x86.cpp: I thought we were discussing how to isolate this with
UseShenandoahGC? It is odd to change upstream LIR_Assembler::leal.

*) src/cpu/aarch64/vm/macroAssembler_aarch64.cpp

It looks to me Shenandoah interception is cleaner with early return like this:

 void MacroAssembler::load_heap_oop(Register dst, Address src)
 {
 #if INCLUDE_ALL_GCS
   if (UseShenandoahGC) {
     ShenandoahBarrierSetAssembler::bsasm()->load_heap_oop(this, dst, src);
     return;
   }
 #endif

   if (UseCompressedOops) {
     ldrw(dst, src);
     decode_heap_oop(dst);
   } else {
     ldr(dst, src);
   }
 }

*) src/cpu/x86/vm/macroAssembler_x86.cpp: same as above. Should just use early return.

*) src/cpu/x86/vm/shenandoahBarrierSetAssembler_x86.cpp: bad indent at L437, L468.

 430 static void load_at(MacroAssembler* masm, Register dst, Address src) {
 431 #ifdef _LP64
 432   if (UseCompressedOops) {
 433     __ movl(dst, src);
 434     __ decode_heap_oop(dst);
 435   } else
 436 #endif
 437     __ movptr(dst, src);
 438 }

 461 static void load_at_not_null(MacroAssembler* masm, Register dst, Address src) {
 462 #ifdef _LP64
 463   if (UseCompressedOops) {
 464     __ movl(dst, src);
 465     __ decode_heap_oop_not_null(dst);
 466   } else
 467 #endif
 468     __ movptr(dst, src);
 469 }

*) src/share/vm/c1/c1_LIRGenerator.cpp, I do not understand this change:

-    if (is_volatile && !needs_patching) {
-      volatile_field_load(address, tmp, info);
+    if (is_volatile) {
+      volatile_field_load(addr->as_address_ptr(), tmp, info);
     } else {
-      LIR_PatchCode patch_code = needs_patching ? lir_patch_normal : lir_patch_none;
-      __ load(address, tmp, info, patch_code);
+      __ load(addr->as_address_ptr(), tmp, info);
     }
     if (is_volatile && os::is_MP()) {
       __ membar_acquire();

Does this mean we always call load(..., lir_patch_none), even when need_patching is true?

*) src/share/vm/gc_implementation/shenandoah/shenandoahBarrierSetC1.cpp, this is odd:

 108 #ifdef AARCH64
 109       // AArch64 expects double-size register.
 110       obj_reg = gen->new_pointer_register();
 111 #else
 112       // x86 expects single-size register.
 113       obj_reg = gen->new_register(T_OBJECT);
 114 #endif

I thought we eliminated it with:
  https://hg.openjdk.java.net/jdk/jdk/rev/732


-- 
Thanks,
-Aleksey