Request reviews (L): 6895383: JCK test throws NPE for method compiled with Escape Analysis

[Tom Rodriguez]

I see what appear to be 4 copies of the Opcode() == Op_ClearArray logic.  Any chance of making one copy to be shared?

escape.cpp:

for the "// push allocation's users on appropriate worklist" code, shouldn't the if/then/else chain be terminated by an else ShouldNotReachHere or are there cases which don't have to be handled?  If you put that in then you want to remove the ifdef ASSERT around the use->is_MergeMem case.

at line 1158, shouldn't the comment just be:

// we don't need to do anything, but the users must be pushed

Otherwise I think this looks ok.

tom

On Nov 18, 2009, at 9:58 AM, Vladimir Kozlov wrote:

> 
> http://cr.openjdk.java.net/~kvn/6895383/webrev.04
> 
> Fixed 6895383: JCK test throws NPE for method compiled with Escape Analysis
> 
> Problem:
> EA misses checks for MemBar nodes when looking for following
> MergeMem nodes. As result it did not add memory slice for
> the volatile store (followed by several MemBar nodes)
> in MergeMem of uncommon trap.
> So when the method deoptimized and eliminated object reallocated
> the NULL is stored to the corresponding field.
> 
> Solution:
> Collect all MergeMem nodes at the beginning of EA when it walks
> through Ideal graph instead of searching MergeMem nodes by going
> down trough memory during memory splitting for non escaping objects.
> Also check for general MemBar nodes instead of only Initialize nodes
> during memory splitting.
> 
> I also did next optimizations/fixes:
> 
> 1. Check for SafePointScalarObject nodes to avoid printing empty lines
>   in PrintOptoAssembly output since they don't have corresponding mach nodes.
> 
> 2. Eliminate volatile MemBars nodes for stores into a scalar replaced objects.
>   I noticed that we still generate membars even if the object is eliminated.
>   For that I added the Precedent edge to corresponding Store node.
> 
> 3. The check for ClearArray node is missing when searching for
>   a better memory edge during EA, memory optimization and an object
>   scalar replacement. We can't bypass it since it is the part of object
>   initialization (zeroing). I will try to factor it into separate function
>   before push.
> 
> 4. Add missing checks for string intrinsic nodes to adjust the escape state
>   (non scalar replaceable) of char[] arrays they use.
> 
> 5. Move code for searching objects which are not scalar replaceable to separate
>   method verify_escape_state() since the code became too large. Add a simple
>   control flow check to find initializing store for oop fields.
> 
> Reviewed by:
> 
> Fix verified (y/n): y, test
> 
> Other testing:
> JPRT, CTW (32 and 64bit)
> 
>