Regarding 8033626: C2: Merging exception state: reexecute=true vs reexecute=false

[Vladimir Ivanov]

The problem is with exception state of two branches [1].
Exception on fast path comes from GraphKit::new_instance during 
intrinsic construction in LibraryCallKit::inline_native_clone. It is 
executed under should_reexecute=true. PreserveReexecuteState doesn't 
touch exception state, so kit.add_exception_states_from(new_jvms) (@ 
callGenerator.cpp:703) tries to accumulate incompatible exceptions from 
fast & slow paths.

Best regards,
Vladimir Ivanov

[1] FAST PATH (intrinsic):

JVMS depth=1 loc=5 stk=9 arg=10 mon=14 scalar=14 end=14 mondepth=0 sp=1 
bci=12 reexecute=false method=virtual jobject

Exception state:

JVMS depth=1 loc=5 stk=9 arg=9 mon=14 scalar=14 end=14 mondepth=0 sp=0 
bci=12 reexecute=true method=virtual jobject

SLOW PATH

JVMS depth=1 loc=5 stk=9 arg=10 mon=14 scalar=14 end=14 mondepth=0 sp=1 
bci=12 reexecute=false method=virtual jobject

Exception state:

JVMS depth=1 loc=5 stk=9 arg=9 mon=14 scalar=14 end=14 mondepth=0 sp=0 
bci=12 reexecute=false method=virtual jobject


On 5/29/14 2:18 AM, Vladimir Kozlov wrote:
> PredictedIntrinsicGenerator was added to have a general predicate. For
> example, AES crypto predicate checks klass of an object pointed by a
> field. PredictedCallGenerator checks klass of method's holder only.
>
> Both cases would have the same problem with reexecute. Using
> PredictedIntrinsicGenerator will not help you.
>
> But you should not have this problem at the merge point, because
> reexecute should be reset back after intrinsic is generated:
>
>    { PreserveReexecuteState preexecs(this);
>      jvms()->set_should_reexecute(true);
>
> It is strange that it is still true after PreserveReexecuteState
> destructor.
>
> Vladimir K
>
> On 5/28/14 9:37 AM, Vladimir Ivanov wrote:
>> Hi,
>>
>> I'm looking at 8033626 [1]:
>> "assert(ex_map->jvms()->same_calls_as(_exceptions->jvms())) failed: all
>> collected exceptions must come from the same place".
>>
>> 8014447 [2] introduced new code shape for virtual intrinsifiable methods
>> (Object::hashCode & Object::clone): "invokevirtual Object.clone()" can
>> be represented as a PredictedCallGenerator with LibraryIntrinsic on fast
>> path and VirtualCallGenerator on slow path.
>>
>> The problem arises for Object::clone, because for OOM case intrinsified
>> version requires reexecution (reexecute=true), but slow path (virtual
>> dispatch) doesn't (reexecute=false). Further attempt to finish the
>> diamond and merge 2 exception states fires the assertion.
>>
>> So, I have 2 questions:
>>    (1) I don't see any way to merge reexecute=true & reexecute=false
>> states, so both branches should have the same reexecute status. Am I
>> missing something?
>>
>>    (2) Does PredictedCallGenerator actually support such shape
>> (intrinsic on fast path)? There are 2 call generators:
>> PredictedIntrinsicGenerator & PredictedCallGenerator. What's the
>> difference?
>>
>> I see there's a way to customize predicate in
>> LibraryCallKit::try_to_predicate() for PredictedIntrinsicGenerator, but
>> is it the only reason why PredictedIntrinsicGenerator was introduced?
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> [1] https://bugs.openjdk.java.net/browse/JDK-8033626
>>
>> [2] https://bugs.openjdk.java.net/browse/JDK-8014447
>>      "Object.hashCode intrinsic breaks inline caches"