RFR(L): JEP165: Compiler Control

[Volker Simonis]

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>
wrote:

> Hi Roland,
>
> On 2015-09-28 12:02, Roland Westrelin wrote:
>
>> Hi Nils,
>>
>> http://cr.openjdk.java.net/~neliasso/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/20151001/695935fc/attachment-0001.html>