RFR 8251268: Move PhaseChaitin definations from live.cpp to chaitin.cpp

[Vladimir Kozlov]

On 8/11/20 12:15 AM, Christian Hagedorn wrote:
> Hi Clive
> 
> Thanks a lot for taking care of this!
> 
> One last comment: The existing spacing for the verify methods in the .hpp file is wrong. But since there are many more 
> methods with a wrong spacing following it, I leave it up to you if you want to fix it for the verify methods or not. I'm 
> fine with both. Either way, you don't need to send another webrev.
> 
> Otherwise, it looks good to me!

+1

Thanks,
Vladimir K

> 
> Best regards,
> Christian
> 
> On 10.08.20 19:00, Verghese, Clive wrote:
>> Hi Christian,
>>
>> Thank you for the feedback. I have updated the review addressing the comments below.
>>
>> http://cr.openjdk.java.net/~xliu/clive/8251268/01/webrev/
>>
>> Regards,
>> Clive Verghese
>>
>>
>>
>> On 8/6/20, 11:55 PM, "Christian Hagedorn" <christian.hagedorn at oracle.com> wrote:
>>      Hi Clive
>>      The fix looks good to me. It makes sense to move it to chaitin.cpp since
>>      the calls to verify() are also in this file only.
>>      You could fix some minor code style things about the existing code that
>>      you moved while at it:
>>      - You can move the #ifdef ASSERT out of both methods and surround both
>>      methods by one single #ifdef ASSERT since verify()/verify_base_ptrs()
>>      are only called in ASSERT blocks. And add a // ASSERT comment on the
>>      closing #endif to make it more clear. Don't forget to also surround the
>>      declarations in the .hpp file with an ASSERT.
>>      - In verify_base_ptrs():
>>         - L2330: Missing curly braces for the loop
>>         - L2297, 2309, 2316: The asterisk should be at the type: ResourceArea
>>      *a -> ResourceArea* a
>>         - There is a missing space in all asserts after the comma separating
>>      the condition and the failure string
>>      - In verify():
>>         - L2386: Missing space and curly braces for the if statement
>>      Best regards,
>>      Christian
>>      On 07.08.20 01:49, Verghese, Clive wrote:
>>      > Hi,
>>      >
>>      > Requesting review for
>>      >
>>      > Webrev : http://cr.openjdk.java.net/~xliu/clive/8251268/00/webrev/
>>      > JBS : https://bugs.openjdk.java.net/browse/JDK-8251268
>>      >
>>      > The change moves the definition of PhaseChaitin::verify_base_ptrs and PhaseChaitin::verify from live.cpp to 
>> chaitin.cpp
>>      >
>>      > I have tested this builds successfully for both PRODUCT and !PRODUCT.
>>      >
>>      > Ensured that there are no regressions in hotspot:tier1 tests.
>>      >
>>      >
>>      > Regards,
>>      > Clive Verghese
>>      >
>>