RFR 8233389: Add PrintIdeal to compiler directives

[Jorn Vernee]

Thanks Tobias!

Jorn

On 08/11/2019 10:54, Tobias Hartmann wrote:
> 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
>>>>>