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

[Vladimir Kozlov]

Now I understand the problem. I think we need to keep separate an 
exception code which requires re-execution. Currently we don't check for 
should_reexecute() when we merge or search for existing exception code 
(we only compare type).

Vladimir K.

On 5/29/14 2:14 AM, Vladimir Ivanov wrote:
> 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"