[10] [ppc] RFR(XS): 8180612: assert failure due to immediate value out of range

[Schmidt, Lutz]

Thank you, Vladimir!

Regards, Lutz

 
Dr. Lutz Schmidt | SAP JVM | PI  SAP CP Core | T: +49 (6227) 7-42834

 

On 23.05.2017, 17:29, "Vladimir Kozlov" <vladimir.kozlov at oracle.com> wrote:

    x86 changes looks good.
    
    Thanks,
    Vladimir
    
    On 5/23/17 12:29 AM, Schmidt, Lutz wrote:
    > Vladimir, Volker,
    > 
    > Triggered by your suggestions, I have read through the RTM code with extended diligence. What I came up with is this updated/extended
    > webrev:  http://cr.openjdk.java.net/~lucy/webrevs/8180612.01/
    > for bug: https://bugs.openjdk.java.net/browse/JDK-8180612
    > 
    > For both x86 and ppc, I have added ranges to all numeric RTM flags. Their type is now “int”. Could you please have a look and let me know what you don’t like?
    > 
    > Thanks and best regards,
    > Lutz
    > 
    >   
    > 
    > On 19.05.2017, 21:40, "Vladimir Kozlov" <vladimir.kozlov at oracle.com> wrote:
    > 
    >      Actually we need to use 'int' because we do signed arithmetic on them. And put range() restriction for positive values only.
    >      
    >         experimental(int, RTMTotalCountIncrRate, 64,                              \
    >                 "Increment total RTM attempted lock count once every n times")    \
    >                 range(0, max_jint)                                                \
    >      
    >      Vladimir
    >      
    >      On 5/19/17 12:33 PM, Vladimir Kozlov wrote:
    >      > Thank you, Volker
    >      >
    >      > I think all RTM tuning flags should be uint (unsigned 32bit int).
    >      > We did not have int/uint types when RTM was implemented. They were added 2 years ago:
    >      >
    >      > http://hg.openjdk.java.net/jdk9/hs/hotspot/rev/8597e296c18b
    >      >
    >      > Lets change type of RTM flags in all places. I will review and sponsor.
    >      >
    >      > thanks,
    >      > Vladimir
    >      >
    >      > On 5/19/17 12:02 PM, Volker Simonis wrote:
    >      >> Hi Lutz, Vladimir,
    >      >>
    >      >> @Lutz: thanks for fixing this. I think your change looks good.
    >      >>
    >      >> @Vladimir: thanks, but I think we can push this ourselves because it
    >      >> is ppc only.
    >      >>
    >      >> I've also realized that amd64 uses cmpptr() which takes the result of
    >      >> "RTMLockingThreshold / RTMTotalCountIncrRate" as an int32_t. This can
    >      >> be wrong if the result of the division is greater than 32 bit. I'm not
    >      >> sure how relevant that is, but maybe we could either change the types
    >      >> of RTMLockingThreshold and RTMTotalCountIncrRate to int or else fix
    >      >> the compare on amd64 to compare against a full 64 bit value.
    >      >>
    >      >> What do you think Vladimir - maybe do that as a follow up change or do
    >      >> you want to include it here (in which case you'd have to sponsor :) ?
    >      >>
    >      >> Thank you and best regards,
    >      >> Volker
    >      >>
    >      >> On Fri, May 19, 2017 at 6:35 PM, Vladimir Kozlov
    >      >> <vladimir.kozlov at oracle.com> wrote:
    >      >>> Hi Lutz,
    >      >>>
    >      >>> I can sponsor it but someone familiar with PPC have to review the fix.
    >      >>>
    >      >>> Thanks,
    >      >>> Vladimir
    >      >>>
    >      >>>
    >      >>> On 5/19/17 5:45 AM, Schmidt, Lutz wrote:
    >      >>>>
    >      >>>> Hi all,
    >      >>>>
    >      >>>> May I kindly request reviews for this small fix? A voluntary sponsor would
    >      >>>> be great as well!
    >      >>>>
    >      >>>> Bug:    https://bugs.openjdk.java.net/browse/JDK-8180612
    >      >>>> Webrev: http://cr.openjdk.java.net/~lucy/webrevs/8180612.00/
    >      >>>>
    >      >>>> The RTM code generation on ppc relied on RTM-related cmdline parameters to
    >      >>>> provide “well-behaved” values only. At least one jtreg test breaks this
    >      >>>> assumption. The fix makes code generation adapt to actual parameter values.
    >      >>>>
    >      >>>> Thanks,
    >      >>>> Lutz
    >      >>>>
    >      >>>>
    >      >>>>
    >      >>>
    >      
    > 
    > 
    > 
    > 
    >