RFR 8233389: Add PrintIdeal to compiler directives

[Vladimir Ivanov]

> http://cr.openjdk.java.net/~jvernee/print_ideal/webrev.02/

Looks good.

> W.r.t. usefulness of PrintIdeal vs PrintIdealGraph; The obvious thing is 
> that PrintIdeal doesn't require IGV, which might be more useful if 
> PrintIdeal were a diagnostic flag instead (as suggested by Nils), so it 
> could be used from a standard JDK build which doesn't come with IGV. 
> Another advantage that comes to mind is that PrintIdeal output is easier 
> to share as text; you can just copy a few lines into the body of an 
> email. That said, I haven't used either of these flags extensively, so I 
> find it hard to judge whether one is clearly better than the other. But, 
> it seems at least unfortunate that we have the PrintIdeal flag, but can 
> not use it in compiler directives to filter the output.

It's a long-standing issue with tracing functionality in C2: both 
options depend on ability to dump Node and Type instances in textual 
form which is absent in product binaries. It would be nice to bundle 
that code in product binaries as well and turn both options into 
diagnostic ones, but nobody have taken care of it yet.

Also, at some point, IGV had a text view of the graph (which was pretty 
close to PrintIdeal output), but I can't find it there anymore.

Best regards,
Vladimir Ivanov

> On 04/11/2019 15:57, Vladimir Ivanov wrote:
>> Hi Jorn,
>>
>> src\hotspot\share\opto\compile.hpp:
>> +   bool                  _print_ideal;           // True if we should 
>> dump node IR for this compilation
>>
>> Since the only usage is in non-product code, I suggest to put 
>> _print_ideal into #ifndef PRODUCT, so you don't need to initialize it 
>> in product build.
>>
>> Also, it'll allow you to just put it on initializer list instead of 
>> doing it in the ctor body (akin to how _trace_opto_output is handled):
>>
>> src\hotspot\share\opto\compile.cpp:
>>
>> Compile::Compile( ciEnv* ci_env,
>> ...
>>   : Phase(Compiler),
>> ...
>>     _has_reserved_stack_access(false),
>> #ifndef PRODUCT
>>     _trace_opto_output(directive->TraceOptoOutputOption),
>> #endif
>>     _has_method_handle_invokes(false),
>>
>>
>> Overall, I don't see much value in PrintIdeal: PrintIdealGraph 
>> provides much more detailed information (even though in XML format) 
>> and IdealGraphVisualizer is better at browsing the graph. The only 
>> thing I'm usually missing is full text dump output on individual nodes 
>> (they are shown pruned in IGV; not sure whether it's IGV fault or the 
>> info is missing in the dump).
>>
>> Best regards,
>> Vladimir Ivanov
>>
>> On 01.11.2019 18:09, Jorn Vernee wrote:
>>> Hi,
>>>
>>> I'd like to add PrintIdeal as a compiler directive in order to enable 
>>> PrintIdeal for only a single method when combining it with the 
>>> 'match' directive.
>>>
>>> Please review the following:
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8233389
>>> Webrev: http://cr.openjdk.java.net/~jvernee/print_ideal/webrev.00/
>>> (Testing = tier1, manual)
>>>
>>> As a heads-up; I'm not a committer on the jdk project, so if this 
>>> sounds like a good idea, I would require a sponsor to push the changes.
>>>
>>> Thanks,
>>> Jorn
>>>