RFR(L): JEP165: Compiler Control
Volker Simonis
volker.simonis at gmail.com
Mon Oct 5 14:43:06 UTC 2015
Hi Nils,
the IGV printing seems to work fine now. Thanks for fixing it!
But I’ve also noticed another change of the previous default behaviour
for printing methods which are not compiled.
Before, if I used the 'compileonly' command and PrintCompilation the
VM only printed the compilation of the requested method. Now, by
default it prints all the methods which have been excluded from the
compilation. I don't know if this was done intentionally, but in my
opinion it is annoying and I'd appreciate if you could restore the
original behaviour (although this could also be worked around by using
the 'quite' command).
This is the old code:
-bool CompilerOracle::should_exclude(methodHandle method, bool& quietly) {
- quietly = true;
...
if (lists[CompileOnlyCommand] != NULL) {
return !lists[CompileOnlyCommand]->match(method);
}
which returned "quietly=true" for methods not in the CompileOnlyCommand list.
This is the new code:
+bool CompilerOracle::should_exclude(methodHandle method) {
+ if (check_predicate(ExcludeCommand, method)) {
+ return true;
}
if (lists[CompileOnlyCommand] != NULL) {
return !lists[CompileOnlyCommand]->match(method);
}
Here, quietly is not touched but instead the new method
should_exclude_quietly() is used later on which returns "quiet=false".
Regards,
Volker
On Mon, Oct 5, 2015 at 4:25 PM, Nils Eliasson <nils.eliasson at oracle.com> wrote:
> New webrev here:
> http://cr.openjdk.java.net/~neliasso/8046155/webrev.06/
>
> Regards
> Nils
>
>
> On 2015-10-05 14:27, Nils Eliasson wrote:
>
> 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>
> 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.
>>
>>
>
>
>
More information about the hotspot-compiler-dev
mailing list