8218721: C1's CEE optimization produces safepoint poll with invalid debug information
authorthartmann
Tue, 19 Feb 2019 08:58:55 +0100
changeset 53806 3bd474c23ee4
parent 53805 f12e86f1b0d6
child 53807 639a36bc8ef1
8218721: C1's CEE optimization produces safepoint poll with invalid debug information Summary: Bail out of CEE if one of the gotos is a safepoint but the if is not. Reviewed-by: vlivanov, mdoerr
src/hotspot/share/c1/c1_Optimizer.cpp
test/hotspot/jtreg/compiler/c1/TestGotoIf.jasm
test/hotspot/jtreg/compiler/c1/TestGotoIfMain.java
--- a/src/hotspot/share/c1/c1_Optimizer.cpp	Tue Feb 19 11:52:19 2019 +0530
+++ b/src/hotspot/share/c1/c1_Optimizer.cpp	Tue Feb 19 08:58:55 2019 +0100
@@ -174,6 +174,12 @@
   for_each_phi_fun(t_block, phi, return; );
   for_each_phi_fun(f_block, phi, return; );
 
+  // Only replace safepoint gotos if state_before information is available (if is a safepoint)
+  bool is_safepoint = if_->is_safepoint();
+  if (!is_safepoint && (t_goto->is_safepoint() || f_goto->is_safepoint())) {
+    return;
+  }
+
   // 2) substitute conditional expression
   //    with an IfOp followed by a Goto
   // cut if_ away and get node before
@@ -202,7 +208,7 @@
 
   // append Goto to successor
   ValueStack* state_before = if_->state_before();
-  Goto* goto_ = new Goto(sux, state_before, if_->is_safepoint() || t_goto->is_safepoint() || f_goto->is_safepoint());
+  Goto* goto_ = new Goto(sux, state_before, is_safepoint);
 
   // prepare state for Goto
   ValueStack* goto_state = if_state;
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/compiler/c1/TestGotoIf.jasm	Tue Feb 19 08:58:55 2019 +0100
@@ -0,0 +1,171 @@
+/*
+ * Copyright (c) 2019, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ *
+ */
+
+public class compiler/c1/TestGotoIf version 52:0 {
+    public Field f1:"I";
+    public Field f2:"I";
+    public static Field i:"I";
+
+    Method "<init>":"()V" stack 1 locals 1 {
+        aload_0;
+        invokespecial Method java/lang/Object."<init>":"()V";
+        return;
+    }
+
+    public Method test1:"()I" stack 3 locals 1 {
+        aload_0;
+        getfield Field f1:"I";
+        aload_0;
+        getfield Field f2:"I";
+        iconst_1;
+        isub;
+        // Without the fix, if got eliminated by CEE
+        if_icmpgt Null;
+        iconst_1;
+      Return: stack_frame_type stack1;
+      stack_map int;
+        ireturn;
+      Null: stack_frame_type same;
+        iconst_0;
+        goto Return; // Backbranch (t_goto) with safepoint
+    }
+
+    public Method test2:"()I" stack 3 locals 1 {
+        aload_0;
+        getfield Field f1:"I";
+        aload_0;
+        getfield Field f2:"I";
+        iconst_1;
+        isub;
+        goto Skip;
+      Return: stack_frame_type full;
+      stack_map int;
+        ireturn;
+      Skip: stack_frame_type full;
+      stack_map int, int;
+        // Without the fix, if got eliminated by CEE
+        if_icmpgt Null;
+        iconst_1;
+        goto Return; // Backbranch (f_goto) with safepoint
+      Null: stack_frame_type full;
+      stack_map;
+        iconst_0;
+        goto Return; // Backbranch (t_goto) with safepoint
+    }
+
+    public Method test3:"()I" stack 3 locals 1 {
+        aload_0;
+        getfield Field f1:"I";
+        aload_0;
+        getfield Field f2:"I";
+        iconst_1;
+        isub;
+        goto Skip;
+      Return: stack_frame_type full;
+      stack_map int;
+        ireturn;
+      Null: stack_frame_type full;
+      stack_map;
+        iconst_0;
+        goto Return; // Backbranch (t_goto) with safepoint
+      Skip: stack_frame_type full;
+      stack_map int, int;
+        // If will be eliminated by CEE
+        if_icmpgt Null; // Backbranch (if) with safepoint
+        iconst_1;
+        goto Return; // Backbranch (f_goto) with safepoint
+    }
+
+    public Method test4:"()I" stack 3 locals 1 {
+        aload_0;
+        getfield Field f1:"I";
+        aload_0;
+        getfield Field f2:"I";
+        iconst_1;
+        isub;
+        goto Skip;
+      Null: stack_frame_type full;
+      stack_map;
+        iconst_0;
+      Return: stack_frame_type full;
+      stack_map int;
+        ireturn; 
+      Skip: stack_frame_type full;
+      stack_map int, int;
+        // If will be eliminated by CEE
+        if_icmpgt Null; // Backbranch (if) with safepoint
+        iconst_1;
+        goto Return; // Backbranch (f_goto) with safepoint
+    }
+
+    public Method test5:"()I" stack 3 locals 2 {
+        aload_0;
+        getfield Field f1:"I";
+        aload_0;
+        getfield Field f2:"I";
+        iconst_1;
+        isub;
+        goto Skip;
+      Null: stack_frame_type full;
+      stack_map;
+        iconst_0;
+        goto Return;
+      Skip: stack_frame_type full;
+      stack_map int, int;
+        // If will be eliminated by CEE
+        if_icmpgt Null; // Backbranch (if) with safepoint
+        iconst_1;
+      Return: stack_frame_type full;
+      stack_map int;
+        ireturn; 
+    }
+
+    public Method test6:"()I" stack 4 locals 1 {
+        getstatic Field i:"I";
+      Loop: stack_frame_type full;
+      stack_map int;
+        // Decrement i and exit loop if < 0
+        iconst_0;
+        getstatic Field i:"I";
+        iconst_1;
+        isub;
+        dup;
+        putstatic Field i:"I";
+        if_icmpgt Exit;
+
+        iconst_1;
+        // Without the fix, if got eliminated by CEE
+        if_icmpgt Null;
+        iconst_1;
+        goto Loop; // Backbranch (f_goto) with safepoint
+      Null: stack_frame_type same;
+        iconst_0;
+        goto Loop; // Backbranch (t_goto) with safepoint
+
+      Exit: stack_frame_type full;
+      stack_map int;
+        iconst_0;
+        ireturn; 
+    }
+}
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/hotspot/jtreg/compiler/c1/TestGotoIfMain.java	Tue Feb 19 08:58:55 2019 +0100
@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2019, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8218721
+ * @compile TestGotoIf.jasm
+ * @run main/othervm -XX:TieredStopAtLevel=1 -Xcomp
+ *                   -XX:CompileCommand=compileonly,compiler.c1.TestGotoIf::test*
+ *                   compiler.c1.TestGotoIfMain
+ */
+
+package compiler.c1;
+
+public class TestGotoIfMain {
+    public static void main(String[] args) {
+        TestGotoIf test = new TestGotoIf();
+        test.i = 5;
+        test.test1();
+        test.test2();
+        test.test3();
+        test.test4();
+        test.test5();
+        test.test6();
+    }
+}