RFR(L): JEP165: Compiler Control

[Nils Eliasson]

Hi,

Thanks for finding.

I fixed it by setting the Compile in the IdealGraphPrinter at the 
beginning of a compilation, and then removed the passing of Compile as 
an argument.

Best regards,
Nils Eliasson

On 2015-10-01 16:48, Volker Simonis wrote:
> Hi Nils,
>
> I've just found another problem with the IGV. After your changes it 
> will crash because you changed IdealGraphPrinter::should_print() to 
> use "C->dirset()->IGVPrintLevelOption >= level" but at the call sites 
> where should_print() is used the Compiler field of the IGV printer 
> isn't initialized.
>
> The quick hack attached below, which always passes the compiler object 
> to should_print() fixes the problem, but may you can up with something 
> better?
>
> And what about the InlineMatcherTest.java which I've already objected 
> in my previous mail. It still doesn't seem to work. How is it supposed 
> to be executed?
>
> Regards,
> Volker
>
> diff -r 6fba102266ad src/share/vm/opto/compile.cpp
> --- a/src/share/vm/opto/compile.cpp    Thu Oct 01 16:24:12 2015 +0200
> +++ b/src/share/vm/opto/compile.cpp    Thu Oct 01 16:40:46 2015 +0200
> @@ -838,7 +838,7 @@
>    // Drain the list.
>    Finish_Warm();
>  #ifndef PRODUCT
> -  if (_printer && _printer->should_print(1)) {
> +  if (_printer && _printer->should_print(C, 1)) {
>      _printer->print_inlining(this);
>    }
>  #endif
> diff -r 6fba102266ad src/share/vm/opto/compile.hpp
> --- a/src/share/vm/opto/compile.hpp    Thu Oct 01 16:24:12 2015 +0200
> +++ b/src/share/vm/opto/compile.hpp    Thu Oct 01 16:40:46 2015 +0200
> @@ -691,7 +691,7 @@
>
>    void begin_method() {
>  #ifndef PRODUCT
> -    if (_printer && _printer->should_print(1)) {
> +    if (_printer && _printer->should_print(C, 1)) {
>        _printer->begin_method(this);
>      }
>  #endif
> @@ -710,7 +710,7 @@
>
>
>  #ifndef PRODUCT
> -    if (_printer && _printer->should_print(level)) {
> +    if (_printer && _printer->should_print(C, level)) {
>        _printer->print_method(this, 
> CompilerPhaseTypeHelper::to_string(cpt), level);
>      }
>  #endif
> @@ -727,7 +727,7 @@
>        event.commit();
>      }
>  #ifndef PRODUCT
> -    if (_printer && _printer->should_print(level)) {
> +    if (_printer && _printer->should_print(C, level)) {
>        _printer->end_method();
>      }
>  #endif
> diff -r 6fba102266ad src/share/vm/opto/idealGraphPrinter.cpp
> --- a/src/share/vm/opto/idealGraphPrinter.cpp    Thu Oct 01 16:24:12 
> 2015 +0200
> +++ b/src/share/vm/opto/idealGraphPrinter.cpp    Thu Oct 01 16:40:46 
> 2015 +0200
> @@ -669,7 +669,7 @@
>  // Print current ideal graph
>  void IdealGraphPrinter::print(Compile* compile, const char *name, 
> Node *node, int level, bool clear_nodes) {
>
> -  if (!_current_method || !_should_send_method || 
> !should_print(level)) return;
> +  if (!_current_method || !_should_send_method || 
> !should_print(compile, level)) return;
>
>    this->C = compile;
>
> @@ -722,8 +722,8 @@
>  }
>
>  // Should method be printed?
> -bool IdealGraphPrinter::should_print(int level) {
> -  return C->dirset()->IGVPrintLevelOption >= level;
> +bool IdealGraphPrinter::should_print(Compile* compile, int level) {
> +  return compile->dirset()->IGVPrintLevelOption >= level;
>  }
>
>  extern const char *NodeClassNames[];
> diff -r 6fba102266ad src/share/vm/opto/idealGraphPrinter.hpp
> --- a/src/share/vm/opto/idealGraphPrinter.hpp    Thu Oct 01 16:24:12 
> 2015 +0200
> +++ b/src/share/vm/opto/idealGraphPrinter.hpp    Thu Oct 01 16:40:46 
> 2015 +0200
> @@ -133,7 +133,7 @@
>    void print_method(Compile* compile, const char *name, int level=1, 
> bool clear_nodes = false);
>    void print(Compile* compile, const char *name, Node *root, int 
> level=1, bool clear_nodes = false);
>    void print_xml(const char *name);
> -  bool should_print(int level);
> +  bool should_print(Compile* compile, int level);
>  };
>
>  #endif
> diff -r 6fba102266ad src/share/vm/opto/parse2.cpp
> --- a/src/share/vm/opto/parse2.cpp    Thu Oct 01 16:24:12 2015 +0200
> +++ b/src/share/vm/opto/parse2.cpp    Thu Oct 01 16:40:46 2015 +0200
> @@ -2379,7 +2379,7 @@
>
>  #ifndef PRODUCT
>    IdealGraphPrinter *printer = IdealGraphPrinter::printer();
> -  if (printer && printer->should_print(1)) {
> +  if (printer && printer->should_print(C, 1)) {
>      char buffer[256];
>      sprintf(buffer, "Bytecode %d: %s", bci(), Bytecodes::name(bc()));
>      bool old = printer->traverse_outs();
>
> On Thu, Oct 1, 2015 at 1:47 PM, Nils Eliasson 
> <nils.eliasson at oracle.com <mailto:nils.eliasson at oracle.com>> wrote:
>
>     Hi Roland,
>
>     On 2015-09-28 12:02, Roland Westrelin wrote:
>
>         Hi Nils,
>
>             http://cr.openjdk.java.net/~neliasso/8046155/webrev.03/
>             <http://cr.openjdk.java.net/%7Eneliasso/8046155/webrev.03/>
>
>         What does CompileCommandCompatibility do?
>
>
>     Control backwards compatibility with CompileCommand for testing
>     purposes. I 'll change the name to the moee descriptive
>     CompilerDirectivesIgnoreCompileCommands (default false).
>
>
>         vm_operations.cpp
>
>         Is that extra include really needed?
>
>     No, removed.
>
>
>         CompilerQueueTest.java
>
>         Is that extra import really needed?
>
>     No, removed.
>
>
>         block.hpp
>
>         You don’t need that ciEnv here, right?
>
>     No, left over from when the directives was stored in the viEnv.
>
>
>         compileBroker.cpp
>
>         1717   should_break = dirset->BreakAtExecuteOption ||
>         task->check_break_at_flags();
>
>         doesn’t seem to be used
>
>         Except for those minor issues, looks good to me.
>
>     Fixed.
>
>
>
>             - Won't build c1 only or shark only.
>
>         You can’t push if c1 only doesn’t build, right?
>
>
>     It was just a temporary glitch that is fixed now.
>
>     Thanks,
>     Nils
>
>
>         Roland.
>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20151005/dbcea355/attachment-0001.html>