RFR(S): 8179618: Fixes for range of OptoLoopAlignment and Inlining flags

[Lindenmaier, Goetz]

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.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20170512/ee280fcd/attachment.html>