[PING] RFR(M): 8207343: Automate vtable/itable stub size calculation

[Schmidt, Lutz]

Dear Community,
are there any praises, objections, questions, comments, or even reviews for this change?
Thanks for considering!
Lutz

On 13.08.18, 12:47, "Schmidt, Lutz" <lutz.schmidt at sap.com> wrote:

    Hi Boris, 
    
    back from vacation I'd like to elaborate a bit more on your comments.
    
    Based on your input, I have changed the description in VtableStub::pd_code_alignment() to read
      "ARM32 cache line size is not an architected constant. We just align on word size."
    
    The description for aarch64 was adapted accordingly:
      "aarch64 cache line size is not an architected constant. We just align on 4 bytes (instruction size)."
    
    With respect to the variable name, I just harmonized the naming across all platforms. I would appreciate if you could live with it. Another option would be to just return a value, without using any variable name. I don't like that too much, but if the community prefers it...
    
    I have updated http://cr.openjdk.java.net/~lucy/webrevs/8207343.01 in-place with just the two comment lines modified. 
    
    Thank you,
    Lutz
    
    On 07.08.18, 22:12, "Schmidt, Lutz" <lutz.schmidt at sap.com> wrote:
    
        Hi Boris,
        thanks for looking at this. I will respond to your comments in more detail next week. I'm on vacation this week.
        Thanks,
        Lutz
        
        On 05.08.18, 19:08, "Boris Ulasevich" <boris.ulasevich at bell-sw.com> wrote:
        
            Hi Lutz,
            
               I have run jtreg with your change and do not see new fails (test that 
            fails on aarch64 is excluded for 32 bit platforms).
            
               I am OK with your change (I'm not a reviewer). But description and 
            variable name in VtableStub::pd_code_alignment function looks strange 
            for me. Raspberry Pi2 ARM1176JZF-S processor has a cache line length of 
            32 bytes, and, as I know, icache line size is not a constant for ARM32 
            architecture.
            
            regards,
            Boris
            
            On 02.08.2018 15:02, Schmidt, Lutz wrote:
            > Hi Zhongwei,
            > 
            > thank you for testing aarch64. Given my lack of expertise, I am surprised there are only those two issues.
            > Ad 1.: fixed. The declaration just a few lines above was forgotten to adapt.
            > Ad 2.: adapted. I pushed the estimate to 152 bytes. I expected this value would require some adjustment.
            > 
            > Please find a new webrev with the changes at http://cr.openjdk.java.net/~lucy/webrevs/8207343.01
            > 
            > Best Regards,
            > Lutz
            > 
            > On 02.08.18, 04:45, "Zhongwei Yao" <Zhongwei.Yao at arm.com> wrote:
            > 
            >      Hi, Lutz,
            >      
            >      I have tested it on aarch64 by running jtreg tests. And find two tiny issues in vtableStubs_aarch64.cpp's VtableStubs::create_itable_stub function:
            >      1. typecheckSize on line 212 is not defined.
            >      2. estimate on line 211 is not large enough, I get 148 in gc/g1/TestFromCardCacheIndex.java case. Here is the assertion failure from that case:
            >          assert(slop_delta >= 0) failed: itable #3: Code size estimate (140) for lookup_interface_method too small, required: 148
            >      
            >      However, I don't have tested the modification in vtableStubs_arm.cpp due to I don't have an arm32 environment at hand.
            >      
            >      --
            >      Best regards,
            >      Zhongwei
            >      
            >      ________________________________________
            >      From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net> on behalf of Schmidt, Lutz <lutz.schmidt at sap.com>
            >      Sent: Monday, July 30, 2018 3:57:05 PM
            >      To: hotspot-compiler-dev at openjdk.java.net
            >      Subject: RFR(M): 8207343: Automate vtable/itable stub size calculation
            >      
            >      Dear all,
            >      
            >      may I please request reviews for this change:
            >      
            >      Bug:    https://bugs.openjdk.java.net/browse/JDK-8207343
            >      Webrev: http://cr.openjdk.java.net/~lucy/webrevs/8207343.00/
            >      
            >      With this change, I try to get rid of the a-priory size guessing for vtable and itable stubs. Please refer to the bug description for all the details. I didn't want to duplicate that text.
            >      
            >      ARM and AARCH64 help requested!
            >      The edits in vtableStubs_aarch64.cpp and vtableStubs_arm.cpp are made blindfolded. I am neither an ARM expert nor do I have build or test hardware available. I would be very grateful if one of the ARM gurus could please fill in for me.
            >      
            >      Thank you!
            >      Lutz
            >      
            >      
            >      
            >