RFR: 8303970: C2 can not merge homogeneous adjacent two If

[Tobias Hartmann]

On Fri, 10 Mar 2023 14:37:06 GMT, Yi Yang <yyang at openjdk.org> wrote:

> Hi, can I have a review for this patch? It adds new Identity for BoolNode to lookup homogenous integer comparison, i.e. `Bool (CmpX a b)` is identity to `Bool (CmpX b a)`, in this way, we are able to merge the following two "identical" Ifs, which is not before.
> 
> 
>     public static void test(int a, int b) { // ok, identical ifs, apply split_if
>         if (a == b) {
>             int_field = 0x42;
>         } else {
>             int_field = 42;
>         }
>         if (a == b) {
>             int_field = 0x42;
>         } else {
>             int_field = 42;
>         }
>     }
> 
>     public static void test(int a, int b) { // do nothing
>         if (a == b) {
>             int_field = 0x42;
>         } else {
>             int_field = 42;
>         }
>         if (b == a) {
>             int_field = 0x42;
>         } else {
>             int_field = 42;
>         }
>     }

I executed some quick testing and `applications/ctw/modules/jdk_internal_le.java` fails with `-ea -esa -XX:CompileThreshold=100 -XX:+UnlockExperimentalVMOptions -server -XX:-TieredCompilation`:


# A fatal error has been detected by the Java Runtime Environment:
#
#  Internal Error (/workspace/open/src/hotspot/share/opto/loopnode.hpp:1136), pid=1175080, tid=1175153
#  assert(n != nullptr) failed: Bad immediate dominator info.
#
# JRE version: Java(TM) SE Runtime Environment (21.0) (fastdebug build 21-internal-LTS-2023-03-10-1459266.tobias.hartmann.jdk2)
# Java VM: Java HotSpot(TM) 64-Bit Server VM (fastdebug 21-internal-LTS-2023-03-10-1459266.tobias.hartmann.jdk2, mixed mode, sharing, compressed oops, compressed class ptrs, g1 gc, linux-amd64)
# Problematic frame:
# V  [libjvm.so+0x14aa3ab]  PhaseIdealLoop::idom_no_update(unsigned int) const+0x17b

Current CompileTask:
C2:   5953 1184    b        jdk.internal.org.jline.reader.impl.LineReaderImpl::viYankTo (56 bytes)

Stack: [0x00007f50104c5000,0x00007f50105c6000],  sp=0x00007f50105bffa0,  free space=1003k
Native frames: (J=compiled Java code, j=interpreted, Vv=VM code, C=native code)
V  [libjvm.so+0x14aa3ab]  PhaseIdealLoop::idom_no_update(unsigned int) const+0x17b  (loopnode.hpp:1136)
V  [libjvm.so+0x14e7f6f]  PhaseIdealLoop::split_if_with_blocks_post(Node*)+0x32f  (loopnode.hpp:1149)
V  [libjvm.so+0x14e8869]  PhaseIdealLoop::split_if_with_blocks(VectorSet&, Node_Stack&)+0x209  (loopopts.cpp:1828)
V  [libjvm.so+0x14d9aff]  PhaseIdealLoop::build_and_optimize()+0x12df  (loopnode.cpp:4520)
V  [libjvm.so+0xb2f661]  PhaseIdealLoop::optimize(PhaseIterGVN&, LoopOptsMode)+0x261  (loopnode.hpp:1111)
V  [libjvm.so+0xb2ae1f]  Compile::Optimize()+0xe2f  (compile.cpp:2149)
V  [libjvm.so+0xb2d47a]  Compile::Compile(ciEnv*, ciMethod*, int, Options, DirectiveSet*)+0x191a  (compile.cpp:833)
V  [libjvm.so+0x939857]  C2Compiler::compile_method(ciEnv*, ciMethod*, int, bool, DirectiveSet*)+0x4e7  (c2compiler.cpp:113)
V  [libjvm.so+0xb3ad6c]  CompileBroker::invoke_compiler_on_method(CompileTask*)+0xa7c  (compileBroker.cpp:2265)
V  [libjvm.so+0xb3bbe0]  CompileBroker::compiler_thread_loop()+0x690  (compileBroker.cpp:1944)
V  [libjvm.so+0x108d796]  JavaThread::thread_main_inner()+0x206  (javaThread.cpp:710)
V  [libjvm.so+0x1a92900]  Thread::call_run()+0x100  (thread.cpp:224)
V  [libjvm.so+0x1730fc3]  thread_native_entry(Thread*)+0x103  (os_linux.cpp:740)


Which makes sense because your optimization does not respect dominance, right?

-------------

Changes requested by thartmann (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12978