RFR: 8252505: C1/C2 compiler support for blackholes [v11]

[Aleksey Shipilev]

On 12/1/20 5:28 PM, Vladimir Ivanov wrote:
>> Seems easier to just accept simple match rules in `.ad`, to be honest. I don't think maintainability is improved by hacking in blackhole handling in `Matcher::match_sfpt`.
> 
> What exactly do you see useful for blackhole inside `if(
> sfpt->is_Call())` branch? Everything except setting `tf()` looks
> irrelevant and you can cleanly capture that in MachCallBlackhole
> factory/ctor.

It's not that. ADLC actually creates a subclass that defines Name(), rule(), etc.

We can fake it, but what I am concerned about is introducing a special case for otherwise generic 
Matcher code. We also basically sidestep DFA machinery, and I am not at all sure what is the effect 
at subsequent matches for the incoming edges.

Is there a precedent to create Mach nodes like this? Do we actually know if there are pitfalls? I 
firmly believe that sticking with what Matcher expects of us: *Node -> Mach*Node rules in .ad -- is 
safer all around.

See the example patch:
   https://cr.openjdk.java.net/~shade/8252505/shared-match.patch

It currently fails with:

Current CompileTask:
C2:    251   16 %           compiler.blackhole.BlackholeInstanceSingleArgTest::test_boolean @ 10 (38 
bytes)

#  Internal Error (/home/shade/trunks/jdk/src/hotspot/share/opto/node.hpp:385), pid=614173, tid=614212
#  assert(i < _max) failed: oob: i=-1, _max=0

Stack: [0x00007f2cd4406000,0x00007f2cd4507000],  sp=0x00007f2cd45021c0,  free space=1008k
Native frames: (J=compiled Java code, A=aot compiled Java code, j=interpreted, Vv=VM code, C=native 
code)
V  [libjvm.so+0x1449400]  Matcher::xform(Node*, int)+0x5f0
V  [libjvm.so+0x144eb96]  Matcher::match()+0x4626
V  [libjvm.so+0xa37ff4]  Compile::Code_Gen()+0xb4
V  [libjvm.so+0xa419b9]  Compile::Compile(ciEnv*, ciMethod*, int, bool, bool, bool, bool, 
DirectiveSet*)+0x1949
V  [libjvm.so+0x87e140]  C2Compiler::compile_method(ciEnv*, ciMethod*, int, bool, DirectiveSet*)+0x1e0
V  [libjvm.so+0xa53238]  CompileBroker::invoke_compiler_on_method(CompileTask*)+0xea8
V  [libjvm.so+0xa53df8]  CompileBroker::compiler_thread_loop()+0x5b8
V  [libjvm.so+0x1a1e0a6]  JavaThread::thread_main_inner()+0x266
V  [libjvm.so+0x1a247c4]  Thread::call_run()+0x104
V  [libjvm.so+0x15c33ce]  thread_native_entry(Thread*)+0x11e

I am not sure I want to trade in a few lines of .ad with fixing new matcher bugs.

> And don't forget about ADLC changes. Those can also go away / migrate to
> MachCallBlackhole.

Yes, they could, but we are drilling another hole in the code that is already quite funky.

Let's not.

-- 
Thanks,
-Aleksey