[8u] RFR 8228405: Incorrect format strings in PhaseIdealLoop::rc_predicate

[Andrew John Hughes]

On 18/07/2019 22:55, Aleksey Shipilev wrote:
> This is original 8u fix:
>   https://bugs.openjdk.java.net/browse/JDK-8228405
>   https://cr.openjdk.java.net/~shade/8228405/webrev.01/
> 
> See the bug for discussion how it came to happen this way.
> 
> Testing: tier1-like scenario
> 

It looks like the full story goes along the lines of:

1. JDK-8182299 [0] is committed to OpenJDK 10.

2. JDK-8173770 is developed in secret as part of the July 2017 CPU.

3. When the CPU is pushed to the public tree, it has to be merged with
8182299, but 8173770 deletes the code that 8182299 fixes up:

diff --git a/hotspot/src/share/vm/opto/loopPredicate.cpp
b/hotspot/src/share/vm/opto/loopPredicate.cpp
--- a/hotspot/src/share/vm/opto/loopPredicate.cpp
+++ b/hotspot/src/share/vm/opto/loopPredicate.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2014, Oracle and/or its affiliates. All rights
reserved.
+ * Copyright (c) 2011, 2017, Oracle and/or its affiliates. All rights
reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -662,9 +662,13 @@
   if (offset && (!offset->is_Con() || offset->get_int() != 0)){
     max_idx_expr = new AddINode(max_idx_expr, offset);
     register_new_node(max_idx_expr, ctrl);
-    if (TraceLoopPredicate)
-      if (offset->is_Con()) predString->print("+ %d ", offset->get_int());
-      else predString->print("+ offset ");
+    if (TraceLoopPredicate) {
+      if (offset->is_Con()) {
+        predString->print("+ %d ", offset->get_int());
+      } else {
+        predString->print("+ offset ");
+      }
+    }
   }

   CmpUNode* cmp = new CmpUNode(max_idx_expr, range);

and introduces three new cases of the same thing.

Thus, in this case, the merge is probably the right place to do this
(even though it's a bit ugly). The alternative would have been to
regress part of 8182299 in 10 during the merge, and then re-apply it in
the form of the patch you've posted here.

Of course, 8u doesn't have 8182299, so it was never fixed in either
version of loopPredicate.cpp.

In short, patch looks fine to me. Simon has actually proposed a backport
of 8182299 that I haven't had chance to look at yet, so I don't know if
he also covered this or not.

[0] https://bugs.openjdk.java.net/browse/JDK-8182299
-- 
Andrew :)

Senior Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: ed25519/0xCFDA0F9B35964222 (hkp://keys.gnupg.net)
Fingerprint = 5132 579D D154 0ED2 3E04  C5A0 CFDA 0F9B 3596 4222
https://keybase.io/gnu_andrew