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

[Verghese, Clive]

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
    >