RFR (11u, XXL): Upstream/backport Shenandoah to JDK11u - the review thread

[Aditya Mandaleeka]

Hey Roman,

Thanks for addressing the comments! Just following up on one of them:

> 
> >     The outermost block has been relaxed to be under `#if
> > INCLUDE_G1GC || INCLUDE_SHENANDOAHGC`, but I think
> >     that last line above (the call to
> > G1ThreadLocalData::dirty_card_queue_buffer_offset()) won't compile if
> > G1
> >     is disabled, will it? I haven't verified it myself, but you may
> > want to check that. I assume including
> >     Shenandoah and not G1 is a valid configuration based on the other
> > checks you added.
> > 
> 
> I think you're missing the final #endif after the block that you showed
> here. The one that closes the #if INCLUDE_G1GC || INCLUDE_SHENANDOAHGC.
> I think it's ok. Also notice that
> G1ThreadLocalData::dirty_card_queue_buffer_offset() will never happen
> with Shenandoah, we don't use the dirty_card_queue.

I think I should have explained the issue I see better.

Here is the relevant code from escape.cpp again, after the patch:

        #if INCLUDE_G1GC || INCLUDE_SHENANDOAHGC
                  if ((UseG1GC SHENANDOAHGC_ONLY(|| UseShenandoahGC)) && adr->is_AddP()) {
                    Node* base = get_addp_base(adr);
                    if (base->Opcode() == Op_LoadP &&
                        base->in(MemNode::Address)->is_AddP()) {
                      adr = base->in(MemNode::Address);
                      Node* tls = get_addp_base(adr);
                      if (tls->Opcode() == Op_ThreadLocal) {
                        int offs = (int)igvn->find_intptr_t_con(adr->in(AddPNode::Offset), Type::OffsetBot);
        #if INCLUDE_G1GC && INCLUDE_SHENANDOAHGC
                        const int buf_offset = in_bytes(UseG1GC ? G1ThreadLocalData::satb_mark_queue_buffer_offset()
                                                                : ShenandoahThreadLocalData::satb_mark_queue_buffer_offset());
        #elif INCLUDE_G1GC
                        const int buf_offset = in_bytes(G1ThreadLocalData::satb_mark_queue_buffer_offset());
        #else
                        const int buf_offset = in_bytes(ShenandoahThreadLocalData::satb_mark_queue_buffer_offset());
        #endif
                        if (offs == buf_offset) {
                          break; // G1 pre barrier previous oop value store.
                        }
                        if (offs == in_bytes(G1ThreadLocalData::dirty_card_queue_buffer_offset())) {
                          break; // G1 post barrier card address store.
                        }
                      }
                    }
                  }
        #endif

That last call to G1ThreadLocalData::dirty_card_queue_buffer_offset() is guarded only by
the `#if INCLUDE_G1GC || INCLUDE_SHENANDOAHGC` so it can be included in the compilation
unit even when INCLUDE_G1GC is off (as long as INCLUDE_SHENANDOAHGC is on), and thus cause
a build break. I think that part needs to also be guarded under #if INCLUDE_G1GC.

-Aditya