Some C2 fixes, cleanup and improvements to handling of write barriers
Roland Westrelin
rwestrel at redhat.com
Tue Jun 21 09:19:48 UTC 2016
http://cr.openjdk.java.net/~roland/write-barrier-loopopts/webrev.00/
The main change here is that looking for a dominating memory barrier now
also happens during loop optimizations. It's both a performance
optimization (because it can handle 2 barriers that are far from each
others) and a fix. The current code handles the following example:
if (..) {
write barrier for obj
} else {
read barrier for obj
}
// memory phi here that merges memory proj from the write barrier and
// memory state before the if
by simply replacing the read barrier with the write barrier. Then the
write barrier is before the if and it affects the memory state of both
branches so the phi after the if/else should be updated as well. But the
current code fails to do that.
This change fixes that problem by:
1) At IGVN time, only handling write barriers that dominates (and not
write barriers whose memory dominates)
2) At loop opts time, handling write barriers whose memory dominates and
fixing the memory graph if required
I also changed ShenandoahBarrierNode::dominates_memory_impl and
ShenandoahReadBarrierNode::dominates_memory_rb_impl so they are no
longer recursive (to not blow the stack up) as is usual practice in c2
code. Because dominating write barriers are now also handled in loop
optimizations I set a depth limit for
ShenandoahBarrierNode::dominates_memory_impl. IGVN transformations are
expected to only look around the node being processed and be applied
frequently so they should stay inexpensive. I also added an argument to
ShenandoahBarrierNode::dominates_memory_impl and
ShenandoahReadBarrierNode::dominates_memory_rb_impl so during parsing we
don't cross Phis which I don't think is safe.
I also dropped the change to phaseX.cpp. I don't think that change would
be accepted upstream. Value is also called during PhaseCCP and so a
similar hack would have been required
there. ShenandoahBarrierNode::Value() now drops the constant part of the
type which should lead to the same result in the end because Identity
already handles the constant barrier case and causes the barrier to go
away.
When a write barrier can be removed, to have its memory projection go
away, rather than drop the projection:
Node* ShenandoahWBMemProjNode::Identity(PhaseGVN* phase) {
...
if (wb->as_ShenandoahBarrier()->Identity_impl(phase) != wb) {
// If the parent write barrier would go away, make this mem proj go away first.
// Poke parent to give it a chance to go away too.
phase->igvn_rehash_node_delayed(wb);
return wb->in(ShenandoahBarrierNode::Memory);
}
and then expects:
Node* ShenandoahBarrierNode::Identity(PhaseGVN* phase) {
Node* replacement = Identity_impl(phase);
if (replacement != this) {
// If we have a memory projection, we first need to make it go away.
Node* mem_proj = find_out_with(Op_ShenandoahWBMemProj);
if (mem_proj != NULL) {
phase->igvn_rehash_node_delayed(mem_proj);
return this;
}
}
return replacement;
}
to cause the barrier to be optimized out. I simply made
ShenandoahWriteBarrierNode::Identity() kill the projection. The risk I
see in the current code is that ShenandoahWBMemProjNode::Identity() sees
the barrier can be dropped and makes the projection go away, enqueues
the barrier so it's removed, then some other nodes are processed by igvn
that for some reason ressurects the barrier or makes the Identity_impl()
call not find a replacement and finally
ShenandoahBarrierNode::Identity() is called, the barrier doesn't go away
but it doesn't have a projection any more.
Roland.
More information about the shenandoah-dev
mailing list