RFR(L): JEP165: Compiler Control
Nils Eliasson
nils.eliasson at oracle.com
Tue Oct 6 11:54:39 UTC 2015
Hi Volker,
Yes, this is a difference. The old implementation makes difference
between compileonly and exclude, and that it is difficult keeping that
distinction and at the same time create a new compatibility layer from
the new.
A problem is that if I mute it by default, then there is no way to turn
it on. (Only a quiet command.) That problem remains for the old
implementation.
Regards,
Nils
On 2015-10-05 16:43, Volker Simonis wrote:
> 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