RFR 8233389: Add PrintIdeal to compiler directives

[Tobias Hartmann]

Hi Jorn,

looks good to me too. I'll sponsor.

Best regards,
Tobias

On 05.11.19 17:54, Vladimir Ivanov wrote:
> 
>> 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
>>>>