On Wed, 11 Nov 2020 11:05:47 GMT, Vladimir Ivanov <vliva...@openjdk.org> wrote:
>> Maurizio Cimadamore has updated the pull request incrementally with 10
>> additional commits since the last revision:
>>
>> - Merge pull request #7 from JornVernee/Additional_Review_Comments
>>
>> Additional review comments
>> - Revert System.java changes
>> - Set copyright year for added files to 2020
>> - Check result of AttachCurrentThread
>> - Sort includes alphabetically
>> - Relax ret_addr_offset() assert
>> - Extra space after if
>> - remove excessive asserts in ProgrammableInvoker::invoke_native
>> - Remove os::is_MP() check
>> - remove blank line in thread.hpp
>
> src/hotspot/share/opto/machnode.cpp line 831:
>
>> 829: st->print("%s ",_name);
>> 830: st->print("_arg_regs: ");
>> 831: _arg_regs.print_on(st);
>
> It doesn't print any useful info: `_arg_regs:
> AllocatedObj(0x000000011cf5cbe8)`. Please, improve it.
Ok, I added printing to GrowableArray at some point, but seems that this was
removed in a merge maybe.
> src/hotspot/share/opto/graphKit.cpp line 2665:
>
>> 2663: for (uint vm_ret_pos = 0; vm_ret_pos < n_returns; vm_ret_pos++) {
>> 2664: if (new_call_type->range()->field_at(TypeFunc::Parms +
>> vm_ret_pos) == Type::HALF) {
>> 2665: // FIXME is this needed?
>
> Why do you need the projection at all? Please, clarify and remove `FIXME`
> comment.
Was a leftover. Will remove
> src/hotspot/share/opto/graphKit.cpp line 2675:
>
>> 2673: // Unpack native results if needed
>> 2674: // Need this method type since it's unerased
>> 2675: switch (nep->method_type()->rtype()->basic_type()) {
>
> Are calls returning multiple values supported right now? (From what I'm
> seeing in other places, they are not supported.) If not, then you don't need
> a loop over return values and there are other places where it can simplify
> code.
Yes, multiple returns are not supported currently. Will simplify this.
-------------
PR: https://git.openjdk.java.net/jdk/pull/634
