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