RFR(S): 8080684: PPC64: Fix little-endian build after "8077838: Recent developments for ppc"

[Alexander Smundak]

Looks good to me.

Regards,
Sasha

On Mon, Jun 8, 2015 at 11:31 AM, Volker Simonis
<volker.simonis at gmail.com> wrote:
> Hi Sasha,
>
> you're right - my fix was too complicated. I should have just used
> function_entry() instead of emit_fd() because function_entry() already
> wraps either emit_fd() or pc() depending on the platform. That's
> exactly what I've done in the new patch:
>
> http://cr.openjdk.java.net/~simonis/webrevs/2015/8080684.v2/
>
> As a bonus I've also fixed the Power 8 detection which was broken
> because we issued an illegal 'lqarx' instruction to determine if we're
> on Power 8.
>
> Regards,
> Volker
>
> PS: I was actually able to test this change on a REAL ppc64le machine
> this time :)
>
>
>
> On Wed, May 20, 2015 at 3:25 AM, Alexander Smundak <asmundak at google.com> wrote:
>> It fails to compile because the code still references FileDescriptor,
>> even if ABI_ELFv2 is defined.
>> The revised patch which succeeds is here:
>> http://cr.openjdk.java.net/~asmundak/8080684/hotspot/webrev
>> (it's for jdk8u-dev branch)
>> but IMHO the proposed change isn't semantically right --
>> if a method is called emit_fd, it should create a FileDescriptor.
>> I am not sure that rationale behind this patch is right -- a
>> compile-time error  is usually easy to fix and verify that the semantics
>> is correct while fixing it, whereas runtime error usually requires
>> more effort.
>>
>>
>> On Tue, May 19, 2015 at 9:41 AM, Volker Simonis
>> <volker.simonis at gmail.com> wrote:
>>> Hi,
>>>
>>> can I please get a review for the following fix of the ppc64le build.
>>>
>>> @Sasha, Tiago: I would be also especially interested if somebody with
>>> a little-endian ppc64 system can check if the fix really works there
>>> as I have currently no access to such a system.
>>>
>>> https://bugs.openjdk.java.net/browse/JDK-8080684
>>> http://cr.openjdk.java.net/~simonis/webrevs/2015/8080684/
>>>
>>> Following the details:
>>>
>>> On big-endian ppc64 we need so called 'function descriptors' instead
>>> of simple pointers in order to call functions. That's why the
>>> Assembler class on ppc64 offers an 'emit_fd()' method which can be
>>> used to create such a function descriptor.
>>>
>>> On little-endian ppc64 the ABI was changed (i.e. ABI_ELFv2) and
>>> function descriptors have been removed. That's why the before
>>> mentioned 'emit_fd()' is being excluded from the build with the help
>>> of preprocessor macros if the HotSpot is being build in a little
>>> endian environment:
>>>
>>> #if !defined(ABI_ELFv2)
>>> inline address emit_fd(...)
>>> #endif
>>>
>>> The drawback of this approach is that every call site which uses
>>> 'emit_fd()' has to conditionally handle the case where 'emit_fd()'
>>> isn't present as well. This was exactly the problem with change
>>> "8077838: Recent developments for ppc" which introduced an
>>> unconditional call to 'emit_fd()' in 'VM_Version::config_dscr() which
>>> lead to a build failure on little endian.
>>>
>>> A better approach would be to make 'emit_fd()' available on both,
>>> little- and big-endian platforms and handle the difference internally,
>>> within the function itself. On little-endian, the function will just
>>> return the current PC without emitting any code at all while the
>>> big-endian variant emits the required function descriptor.
>>>
>>> Thank you and best regards,
>>> Volker