[ping] RE: RFR(S): 8179618: Fixes for range of OptoLoopAlignment and Inlining flags

[Tobias Hartmann]

Hi Goetz,

On 23.05.2017 08:29, Lindenmaier, Goetz wrote:
> could someone please sponsor this change?
> Final webrev:
> http://cr.openjdk.java.net/~goetz/wr17/8179618-FlagRanges/webrev.03/

Sure, I'll sponsor it!

Best regards,
Tobias

>> -----Original Message-----
>> From: hotspot-compiler-dev [mailto:hotspot-compiler-dev-
>> bounces at openjdk.java.net] On Behalf Of Lindenmaier, Goetz
>> Sent: Dienstag, 16. Mai 2017 12:08
>> Cc: hotspot-compiler-dev at openjdk.java.net
>> Subject: RE: RFR(S): 8179618: Fixes for range of OptoLoopAlignment and Inlining
>> flags
>>
>> Hi,
>>
>> could someone please sponsor this change?
>> Final webrev:
>> http://cr.openjdk.java.net/~goetz/wr17/8179618-FlagRanges/webrev.03/
>>
>> Thanks,
>>   Goetz
>>
>>> -----Original Message-----
>>> From: Lindenmaier, Goetz
>>> Sent: Freitag, 12. Mai 2017 09:10
>>> To: 'Thomas Stüfe' <thomas.stuefe at gmail.com>
>>> Cc: hotspot-compiler-dev at openjdk.java.net
>>> Subject: RE: RFR(S): 8179618: Fixes for range of OptoLoopAlignment and
>> Inlining
>>> flags
>>>
>>> Hi,
>>>
>>>
>>>
>>> could someone please sponsor? Thanks!
>>>
>>>
>>>
>>> I fixed the print statement.  New webrev anyways:
>>>
>>> http://cr.openjdk.java.net/~goetz/wr17/8179618-FlagRanges/webrev.03/
>>>
>>>
>>>
>>> Best regards,
>>>
>>>   Goetz.
>>>
>>>
>>>
>>> From: Thomas Stüfe [mailto:thomas.stuefe at gmail.com]
>>> Sent: Tuesday, May 09, 2017 7:54 PM
>>> To: Lindenmaier, Goetz <goetz.lindenmaier at sap.com>
>>> Cc: hotspot-compiler-dev at openjdk.java.net
>>> Subject: Re: RFR(S): 8179618: Fixes for range of OptoLoopAlignment and
>> Inlining
>>> flags
>>>
>>>
>>>
>>> Hi Goetz,
>>>
>>>
>>>
>>> On Tue, May 9, 2017 at 4:17 PM, Lindenmaier, Goetz
>>> <goetz.lindenmaier at sap.com <mailto:goetz.lindenmaier at sap.com> > wrote:
>>>
>>> 	Hi Thomas,
>>>
>>> 	thanks for looking at my change.
>>> 	New webrev:
>>> 	http://cr.openjdk.java.net/~goetz/wr17/8179618-
>>> FlagRanges/webrev.02/
>>>
>>> 	> c2_globals.hpp:
>>> 	> -          range(0, max_intx)                                                \
>>> 	> +          range(0, ((intx)MIN2((int64_t)max_intx,(int64_t)(+1.0e10))))      \
>>> 	> 32bit: I would have expected a build warning for the cast. Is it okay
>>> that we can never reach the max value on 32bit?
>>>
>>> 	I double checked that there is no warning in our night builds and on
>>> linuxintel.
>>>
>>> 	> commandLineFlagConstraintsCompiler.cpp:
>>> 	>      CommandLineError::print(verbose,
>>> 	>                              "OptoLoopAlignment (" INTX_FORMAT ") must be "
>>> 	>                              "multiple of NOP size\n");
>>> 	> There is an error here, the print parameter is missing. Would have
>>> expected the compiler to complain, actually - at least the gcc. Again, curious.
>>>
>>> 	Thanks, good catch! The error was there before, but fixed anyways. I
>>> also
>>> 	added the NOP size.
>>>
>>>
>>> +  // Relevant on ppc, s390, sparc. Will be optimized where
>>> +  // addr_unit() == 1.
>>>    if (OptoLoopAlignment % relocInfo::addr_unit() != 0) {
>>>      CommandLineError::print(verbose,
>>>                              "OptoLoopAlignment (" INTX_FORMAT ") must be "
>>> -                            "multiple of NOP size\n");
>>> +                            "multiple of NOP size (" INTX_FORMAT ")\n",
>>> +                            value, relocInfo::addr_unit());
>>>
>>> We are getting there...
>>>
>>>
>>>
>>> addr_unit() returns int, so use %d, not INTX_FORMAT.
>>>
>>>
>>>
>>> Apart from that all is fine. No need for a new webrev.
>>>
>>>
>>>
>>> ..Thomas
>>>
>>>
>>>
>>>
>>> 	Best regards,
>>> 	  Goetz.
>>>
>>>
>>>
>>> 	Kind Regards, Thomas
>>>
>>>
>>> 	On Thu, May 4, 2017 at 12:57 PM, Lindenmaier, Goetz
>>> <mailto:goetz.lindenmaier at sap.com <mailto:goetz.lindenmaier at sap.com> >
>>> wrote:
>>> 	Hi,
>>>
>>> 	This change fixes range handling of a few flags of C2.
>>> 	This should go to jdk10, and later be downported to some
>>> 	update of jdk9.
>>>
>>> 	Please review this change. I please need a sponsor.
>>> 	http://cr.openjdk.java.net/~goetz/wr17/8179618-
>>> FlagRanges/webrev.01/
>>>
>>> 	Class WarmCallInfo limits its values to 1.0e10, but the flags used
>>> 	to set it's fields (HotCallCountThreshold etc.) are limited by max_intx.
>>> 	Using values over 1.0e10 causes assertions in the debug build.
>>>
>>> 	OptoLoopAlignment must be a multiple of nop size, else it's not
>>> 	possible to generate the instructions that go into the pad.
>>> 	On x86 NOP size is 1, so it's no problem.
>>> 	For SPARC, OptoLoopAlignmentConstraintFunc implements a special
>>> 	case for bigger NOPs. This is also needed for s390 and ppc.
>>> 	I just removed the #define, as the code works also on platforms
>>> 	where NOPsize == 1. Actually, it should be optimized by the C
>>> 	compiler in these cases.
>>>
>>> 	Best regards,
>>> 	  Goetz.
>>>
>>>
>