Request for reviews (S): 8002069: Assert failed in C2: assert(field->edge_count() > 0) failed: sanity

[Vladimir Ivanov]

Ah, I see. Missed that difference between develop vs notproduct flags.

Are there any guidelines when a flag should be "notproduct" and when 
"develop"?

Best regards,
Vladimir Ivanov

On 11/7/12 1:38 AM, Vladimir Kozlov wrote:
> Thank you, Vladimir
>
> There are 3 major kinds of flags:
>
> product - available in all versions of VM builds (debug, product,
> optimized)
> develop - also available in all versions of VM build, but declared as
>            a constant value in non-debug builds (product, optimized) -
>            you can change its value on command line only in debug build
> notproduct - declared only in non-product builds (debug,
>               optimized), have to be enclosed by #ifdef ASSERT or
>               #ifndef PRODUCT.
>
> Regards,
> Vladimir K.
>
> Vladimir Ivanov wrote:
>> Vladimir K.,
>>
>> I'd like to clarify one thing with code style.
>>
>> Why do you guard verification code with ASSERT?
>> Isn't it enough to have VerifyConnectionGraph non-product [1], so
>> compiler can just eliminate relevant code as dead?
>>
>> As a use case: it's much easier and less error-prone to enable
>> non-product code guarded by a flag just by changing flag type rather
>> than replacing all relevant asserts. I envision such situations be
>> useful during development/testing/problem diagnostics.
>>
>> Otherwise, the fix looks good.
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> [1] src/share/vm/opto/c2_globals.hpp:460:  notproduct(bool,
>> VerifyConnectionGraph , true,
>>
>> On 11/3/12 2:34 AM, Vladimir Kozlov wrote:
>>> http://cr.openjdk.java.net/~kvn/8002069/webrev/
>>>
>>> The reference to a field on a dead path (which is not eliminated yet) of
>>> bimorphic inlined call has different type (oop) instead of real (int)
>>> field type.
>>>
>>> Added missed type check:
>>>
>>> -          if (store != NULL && store->is_Store()) {
>>> +          if (store != NULL && store->is_Store() &&
>>> +              store->as_Store()->memory_type() == ft) {
>>>
>>> The rest of changes are additional verification and verbose output
>>> during failure which helped me to debug the problem.
>>>
>>> Added compiler regression test.
>>>
>>> Tested with CTW and compiler regression tests.
>>>
>>> Thanks,
>>> Vladimir
>>>