Possible compiler bug: continue is skipped after or-condition

B. Blaser bsrbnd at gmail.com
Wed Feb 19 19:45:48 UTC 2020


Starting with Roland's experiment which is failing some langtools
tests, I tried a similar fix (here under) focusing only on "continue"
to have a clearer idea of its potential impact on C2 (tier1 being OK):

$ cat Demo.java
/* - */ public class Demo {
/* - */     static String[] strings = {"1a", "1b", "2", "3"};
/* - */
/* - */     public static void main(String[] args) {
/* 5 */         for (String s : strings) {
/* 6 */             if (s.equals("1a") || s.equals("1b")) {
/* 7 */                 continue;
/* - */             }
/* 9 */             if (s.equals("2")) {
/* - */                 continue;
/* - */             }
/* - */             System.out.println(s);
/* - */         }
/* - */     }
/* - */ }

$ javac -g Demo.java
$ javap -v Demo.class
8<------8<------
        19: aload         4
        21: ldc           #13                 // String 1a
        23: invokevirtual #15                 // Method
java/lang/String.equals:(Ljava/lang/Object;)Z
        26: ifne          39
        29: aload         4
        31: ldc           #21                 // String 1b
        33: invokevirtual #15                 // Method
java/lang/String.equals:(Ljava/lang/Object;)Z
        36: ifeq          42
        39: goto          63
8<------8<------
      LineNumberTable:
        line 5: 0
        line 6: 19
        line 7: 39
        line 9: 42
        line 10: 52
        line 12: 55
        line 5: 63
        line 14: 69
8<------8<------

$ jdb Demo
Initializing jdb ...
> stop at Demo:7
Deferring breakpoint Demo:7.
It will be set after the class is loaded.
> run
run Demo
Set uncaught java.lang.Throwable
Set deferred uncaught java.lang.Throwable
>
VM Started: Set deferred breakpoint Demo:7

Breakpoint hit: "thread=main", Demo.main(), line=7 bci=39
7    /* 7 */                 continue;

main[1] cont
>
Breakpoint hit: "thread=main", Demo.main(), line=7 bci=39
7    /* 7 */                 continue;

main[1] cont
> 3

The application exited

$ java -Xcomp -XX:-TieredCompilation -XX:CompileCommand=print,Demo.main* Demo
CompileCommand: print Demo.main*
OpenJDK 64-Bit Server VM warning: printing of assembly code is
enabled; turning on DebugNonSafepoints to gain additional output

============================= C2-compiled nmethod ==============================
8<------8<------

03c     B5: #    out( N1 ) <- in( B11 B2 )  Freq: 0.999938
8<------8<------
04b     ret
8<------8<------

050     B6: #    out( B7 ) <- in( B10 ) top-of-loop Freq: 4.49944
8<------8<------

06b     B8: #    out( B22 B9 ) <- in( B7 )  Freq: 4.99949
8<------8<------
097     call,static  java.lang.String::equals
        # Demo::main @ bci:23

09c     B9: #    out( B12 B10 ) <- in( B8 )  Freq: 4.99939
        # Block is sole successor of call
09c     testl   RAX, RAX
09e     je,s   B12  P=0.100000 C=-1.000000

0a0     B10: #    out( B6 B11 ) <- in( B13 B9 B15 )  Freq: 4.99938
8<------8<------
0ad     jl,s   B6    # loop end  P=0.900000 C=-1.000000

0af     B11: #    out( B5 ) <- in( B10 )  Freq: 0.499938
0af     jmp,s   B5

0b1     B12: #    out( B21 B13 ) <- in( B9 )  Freq: 0.499939
8<------8<------
0c3     call,static  java.lang.String::equals
        # Demo::main @ bci:33

0c8     B13: #    out( B10 B14 ) <- in( B12 )  Freq: 0.499929
        # Block is sole successor of call
0c8     testl   RAX, RAX
0ca     jne,s   B10  P=0.900000 C=-1.000000

Some JVM expert (like Andrew) might confirm, but I believe the C2
generated code (x86_64) would still be optimal with a "continue"
elision reversion as here under. As we can see, both bci:23 and bci:33
would go directly to B10 which either starts a new iteration at B6 or
exits through B5.

Should I file a JBS issue to track this potential fix or would we
prefer investigating a much more holistic refactoring in this area?

Thanks,
Bernard


diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Code.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Code.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Code.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Code.java
@@ -1457,6 +1457,10 @@
      */
     public Chain branch(int opcode) {
         Chain result = null;
+//        if (opcode == goto_) {
+//            result = pendingJumps;
+//            pendingJumps = null;
+//        }
         if (opcode == goto_) {
             result = pendingJumps;
             pendingJumps = null;
@@ -1471,6 +1475,18 @@
         return result;
     }

+    public Chain continue_() {
+        Chain result = null;
+        if (isAlive()) {
+            result = new Chain(emitJump(goto_),
+                               result,
+                               state.dup());
+            fixedPc = fatcode;
+            alive = false;
+        }
+        return result;
+    }
+
     /** Resolve chain to point to given target.
      */
     public void resolve(Chain chain, int target) {
diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java
b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/jvm/Gen.java
@@ -1797,7 +1797,8 @@
         Env<GenContext> targetEnv = unwind(tree.target, env);
         code.pendingStatPos = tmpPos;
         Assert.check(code.isStatementStart());
-        targetEnv.info.addCont(code.branch(goto_));
+//        targetEnv.info.addCont(code.branch(goto_));
+        targetEnv.info.addCont(code.continue_());
         endFinalizerGaps(env, targetEnv);
     }

On Mon, 17 Feb 2020 at 22:23, Maurizio Cimadamore
<maurizio.cimadamore at oracle.com> wrote:
>
> Forgot the link:
>
> [1] - https://openjdk.java.net/jeps/303
>
> On 17/02/2020 21:20, Maurizio Cimadamore wrote:
> > They are _generally_ very reliable - to be clear, nothing has changed
> > in this area to make less reliable. You "just" came across a quirk in
> > javac code generation strategy that has been there for the last 20 years.
> >
> > There are other situations where putting a breakpoint won't
> > necessarily result in anything meaningful to occur - for instance, if
> > you put a breakpoint on a local variable declaration. This is not to
> > say that nothing should be done about it _ever_ - but as you said, you
> > lived for 20 years w/o realizing that this hiccup has been there all
> > along (along with other hiccups) - so please also understand a certain
> > reticence in making an abrupt change in the code generation strategy
> > (or worse, supporting multiple code generation strategies) for what
> > feels like something that is annoying, yes, but hardly a breaking issue.
> >
> > As I said in my last email, I'd like to address this as part as a more
> > holistic investigation on how bytecode fidelity is retained from class
> > files. Note that we have features in the pipeline like constant
> > folding (see [1]) which are going to cause an even bigger impact to
> > how source files relates to classfiles. So I'd just like to think this
> > through properly, as I think this needs more thinking and more time
> > than what we've available right now - to fully assess the extent of
> > this and other 'premature' optimizations javac might do to the code,
> > verify that changing code shape won't hurt C1, C2 compilers, and make
> > sure that no relevant functional regression is introduced.
> >
> > Maurizio
> >
> > On 17/02/2020 17:35, Roland Illig wrote:
> >> How would you explain to an average developer that breakpoints are not
> >> reliable? Up to now, they have been very reliable to me, and after more
> >> than 20 years of using Java, this is really the first time that the
> >> breakpoints surprise me.


More information about the compiler-dev mailing list