Problem with testN_mem_reg0 in register allocation.

[Lindenmaier, Goetz]

Hi Vladimir, 

Thanks a lot for your help!
We implemented what you proposed, see webrev. 
We also initialize idealreg2XXXmask completely, as only this allowed us 
to cleanly reproduce the problem.
http://cr.openjdk.java.net/~goetz/webrevs/testN_regalloc_bug/

The fix helped to resolve our current problem.  But it somehow feels
like a quick hack.
Shouldn't already call_catch_cleanup bail out when it makes a Phi for an 
operation that defines RegFlags?  We looked at the memory operations
between the testN and the branch, there is not much besides klass
accesses.  So it could be that gcm thinks block 35 is a good placement, and 
assumes RegAlloc does rematerialization if necessary.  But RegAlloc
fails, as call_catch_cleanup constructed a graph that can not be 
fixed by rematerializations?

In Matcher::find_shared() we can not find code considering anti
dependencies.  We think that code is in LabelRoot.

Best regards,
  Martin & Goetz.


-----Original Message-----
From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com] 
Sent: Montag, 19. August 2013 19:58
To: Lindenmaier, Goetz
Cc: hotspot-compiler-dev at openjdk.java.net
Subject: Re: Problem with testN_mem_reg0 in register allocation.

Goetz, Martin

I would speculate it is scheduled very early by GCM because of memory 
access anti-dependency. Somewhere there is a store or other changes to 
this memory slice. Yes, matcher should not allow such matching. I am not 
sure we have an anti-dependency check when we match (need to look). The 
method which should do that is Matcher::find_shared().

The easiest fix would be in place where we try to spill flag do bailout 
with recompilation without subsuming loads:

     if (C->subsume_loads() == true && !C->failing()) {
       // Retry with subsume_loads == false
       // If this is the first failure, the sentinel string will "stick"
       // to the Compile object, and the C2Compiler will see it and retry.
       C->record_failure(C2Compiler::retry_no_subsuming_loads());

Regards,
Vladimir

On 8/19/13 8:04 AM, Lindenmaier, Goetz wrote:
> Hi,
>
> we are currently analyzing a bug in our jdk7 VM, which is based on
> HS23.  It happens on linux x86_64.
>
> Register allocation tries to spill a RefFlags register, and, as to
> expect, fails on this leading to a SIGSEGV.
>
> I suspect this is a strange interdependency between gcm and
> register allocation
>
> Gcm produces something as the following pattern (I simplified
> to get the essentials).
>
>            Block 35
>              Call
>              209 testN_mem_reg0
>              Catch
>         /              \
>        /                \
>      |/_                _\|
> Block 36             Block 146
>    ...                 ...
>                        159 testP_reg
>                        158 jmpCon(212)
>       \                   /
>        \                 /
>        _\|             |/_
>          Block 39
>            ...
>         /              \
>        /                \
>      |/_                _\|
> Block 140              ...
>    208 jmpCon(209)
>
> Then call_catch_cleanup() repairs the position of 209.
> It adds 1088, 1089 and 1090.
>
>            Block 35
>              Call
>              Catch
>         /              \
>        /                \
>      |/_                _\|
> Block 36               Block 146
>    1089 testN_mem_reg0    1088 testN_mem_reg0
>    ...                    ...
>                           159 testP_reg
>                           158 jmpCon(212)
>       \                   /
>        \                 /
>        _\|             |/_
>          Block 39
>            1090 Phi(1089, 1088)
>            ...
>         /              \
>        /                \
>      |/_                _\|
> Block 140              ...
>    208 jmpCon(1090)
>
> Register allocation now wants to spill the register of 1088.
> This is necessary as 159 needs the flag register.
> Have you seen such a problem before?
>
> We could envision that one the following 3 behaviors is expected and
> should get fixed:
>
> 1. gcm should place node 209 in Block 140.
> 2. Register allocation should be able to properly rematerialize the
> testN_mem_reg0 node (which does allow rematerialization).
>     This would require that it recognizes that the Phi merges twice the
> same value.
> 3. The matcher should not match this node (which matches on a Load and
> Cmp)?
>
> Best reagrds,
> Martin & Goetz.
>