Request for reviews (S): 7026700: regression in 6u24-rev-b23: Crash in C2 compiler in PhaseIdealLoop::build_loop_late_post
Vladimir Kozlov
vladimir.kozlov at oracle.com
Wed Apr 20 15:56:58 PDT 2011
Thank you, Tom
Vladimir
Tom Rodriguez wrote:
> On Apr 20, 2011, at 2:51 PM, Vladimir Kozlov wrote:
>
>> Tom Rodriguez wrote:
>>> On Apr 20, 2011, at 12:11 PM, Vladimir Kozlov wrote:
>>>> This is fix for jdk7.
>>>>
>>>> http://cr.openjdk.java.net/~kvn/7026700/webrev
>>>>
>>>> Fixed 7026700: regression in 6u24-rev-b23: Crash in C2 compiler in PhaseIdealLoop::build_loop_late_post
>>>>
>>>> With EA memory slices should be created explicitly for non-static fields regardless the value of ReduceFieldZeroing flag.
>>> Do these really need to guarded? Couldn't we just always do it?
>> Yes, we can do it always. Which makes raw_mem_only parameter obsolete. Should I remove it?
>
> Sure. The comment for raw_mem_only is incorrect anyway.
>
>>>> LoadNode::split_through_phi() should not check profitability since we want to eliminate loads from non-escaping allocations.
>>> What about this logic?
>>> // Skip the split if the region dominates some control edge of the address.
>>> if (cnt == 3 && !MemNode::all_controls_dominate(address, region))
>>> return NULL;
>>> Why should we give up when cnt == 3 and not others?
>> Yes, it is a bug, we should check all cases. I think my intention was to restrict the call to expensive all_controls_dominate() only for loops. Fixed.
>>
>>>> I also want to disable memory split verification code in EA until the fix for 6984348. It produces false negative results since the verification code does not cover all cases.
>>> Could you normal // comments or use #if 0 instead. It's should have a comment explaining why it's disabled too.
>> Done.
>>
>> I updated webrev.
>
> Looks good.
>
> tom
>
>> Thanks,
>> Vladimir
>>
>>> tom
>
More information about the hotspot-compiler-dev
mailing list