Possible compiler bug: continue is skipped after or-condition

Jan Lahoda jan.lahoda at oracle.com
Wed Feb 19 20:09:45 UTC 2020


FWIW, this may be interesting:
https://mail.openjdk.java.net/pipermail/compiler-dev/2019-September/013678.html

Jan

On 19. 02. 20 20:45, B. Blaser wrote:
> 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