RFR(S): 8192992: Test8007294.java failed: attempted to spill a non-spillable item
vladimir.kozlov at oracle.com
Wed Mar 14 17:42:54 UTC 2018
Very good! Thank you for doing this analysis.
Please, run our usual mach5 set of tests.
On 3/14/18 3:17 AM, Nils Eliasson wrote:
> Hi Vladimir,
> After taking a very close look I found that the anti-dependency checking
> that hoists the testN_mem_reg from the jmpCon is broken, and that the
> hoisting is unnecessary. So this is not a case where we need
> anti-depenency checking for loads before matching.
> Generally the insert_anti_dependences looks good, except the
> store->is_Phi() clause that is full of holes (overly conservative). I
> don't think I fully understand how the graph looks when the clause is
> needed, but it tries to find stores upwards that is otherwise
> unreachable from the downward memory flow search.
> I found these three flaws:
> 1) A Phi in a block that is preceded by a store - even though the store
> is dominated by the loads LCA it will force the testN up! We don't check
> where the stores are located.
> 2) A Phi that consumes the same memory as the load may force it up, even
> though no stores are involved.
> 3) A Phi that consumes a mergemem, which in itself has already has been
> processed and passed as irrelevant, may force the testN up.
> One could add that any predecessor to the phi would have to be a
> store/call to affect the load placement.
> I have also added some additional debugging printouts to the patch.
> // Nils
> On 2018-03-05 18:02, Vladimir Kozlov wrote:
>> Hi Nils,
>> Yes, it is legal workaround but this way you removed all subsuming
>> loads in code.
>> Should we do anti-dependency check for loads during matching when
>> shared nodes are marked?:
>> How expensive would be that?
>> On 3/5/18 7:50 AM, Nils Eliasson wrote:
>>> This patch is a workaround for a scheduling problem encountered in
>>> some rare circumstances. Instead of hitting the assert we retry the
>>> compilation without subsuming loads.
>>> To quote Tobias:
>>> "The crash happens because a testN_mem_reg0 (CmpN(LoadN(mem), NULL))
>>> is scheduled in a different block than its jmpCon user and the
>>> register allocator tries to spill the flag register. The problem is
>>> that PhaseCFG::schedule_late() detects an anti-dependency for the
>>> testN_mem_reg0 on a bottom memory Phi and therefore raises the LCA to
>>> the early block (see PhaseCFG::insert_anti_dependences()) which is
>>> "far away" from its jmpCon user. "
>>> Thanks to Roland who suggested the workaround.
More information about the hotspot-compiler-dev