RFR(S): 8139046: Compiler Control: IVGPrintLevel directive should set PrintIdealGraph

[Volker Simonis]

Hi Xin,

Thanks for addressing this issue. It looks like a nice cleanup. Please find
my further comments inline:

On Tue, Jun 2, 2020 at 10:57 PM Liu, Xin <xxinliu at amazon.com> wrote:

> Hi,
>
> Could you review this webrev? It fixes a minor problem when users only use
> IGVPrintLevel in Compiler Directives.
> Jbs: https://bugs.openjdk.java.net/browse/JDK-8139046
> Webrev: http://cr.openjdk.java.net/~xliu/8139046/00/webrev/
>
> I move "bool should_print(int level)" from idealGraphPrinter to Compile
> because the later has the information.
> In this way, Compile can allocate _printer on demand. If
> Compile::should_print(level) return true,  it guarantees that
> Compile::printer() is not NULL.
> If users pass in CompileCommand="option,Hello::add,intx,IGVPrintLevel,3",
> printer() will only turn on for that compiler thread.
>
>
- Why do you need the extra check for  "method() != NULL":

619     if (should_print(1) && method() != NULL) {
4584   if (should_print(level) && method() != NULL) {

in "Compile::{begin,end_method}". This check wasn't there before. Does it
fix an issue?

- I don't see why you need the additional "need" parameter in
"IdealGraphPrinter::printer()". The function only gets called with "need ==
true" anyway, so I think you can remove it.

- Why did you make

453   IdealGraphPrinter* printer() { return _printer; }

a "const" function?

453   IdealGraphPrinter* printer() const { return _printer; }

I don't think it is required?

- As an additional cleanup, you can change all "should_print(1)" calls to
"should_print()" because "1" is the default parameter anyway.

Besides that, your change looks good.

Thank you and best regards,
Volker

Ran hotspot:tier1 using fastdebug build. Only gtest/GTestWrapper.java
> failed.
> That's another issue. Currently, Openjdk can't execute any gtest because
> of a linkage error.
> Error occurred during initialization of VM
> Unable to load native library:
> /backup/jdk/build/linux-x86_64-server-fastdebug/images/jdk/lib/libjava.so:
> symbol JVM_GetPermittedSubclasses version SUNWprivate_1.1 not defined in
> file libjvm.so with link time reference
>
> Thanks,
> --lx
>
>
>
>