On Thu, 14 Oct 2021 09:46:47 GMT, Andrew Haley <a...@openjdk.org> wrote:

>> This patch expands the newly added system for hsdis backends to include LLVM.
>> 
>> The actual code in hsdis-llvm.cpp is based heavily on the work by @luhenry, 
>> as published in the never integrated PR 
>> https://github.com/openjdk/jdk/pull/392. (I have basically just ripped out 
>> the binutils-based part of it.)
>> 
>> Unfortunately I have not been able to make this work properly on Windows. 
>> With some additional flags I made it compile without complaints, but it 
>> caused hotspot to segfault in `LoadLibrary` (!) in `os::dll_load` when I 
>> tried to load the library. This is somewhat ironic, since the initial 
>> implementation was created by Ludovic for the very purpose of using it on 
>> Windows.
>> 
>> The lack of Windows support in this patch does not mean it is impossible to 
>> get it to work, just that I need to co-operate with someone who has more 
>> experience of compiling LLVM on Windows, and/or are more eager to get this 
>> combination to work.
>
> So I figured out how to build it with `make build-hsdis`. The instructions 
> need to be fixed.
> 
> Two problems: firstly, the library seems to be built with the wrong name, so 
> the runtime doesn't find it. I had to use
> `ln -sf ~/lib/libhsdis.dylib ~/lib/hsdis-aarch64.dylib`
> to get it to work.
> 
> More severely, the disassembly is truncated, so lots of stuff doesn't work. 
> The binutils-based hsdis prints
> 
> 
>  0x00000001105665b4:   stp     wzr, wzr, [x20, #60]        ;*putfield 
> preCounterBlock {reexecute=0 rethrow=0 return_oop=0}
>                                                             ; - 
> com.sun.crypto.provider.GaloisCounterMode$GCMEngine::<init>@80 (line 673)
>                                                             ; - 
> com.sun.crypto.provider.GaloisCounterMode$GCMDecrypt::<init>@8 (line 1346)
>                                                             ; - 
> com.sun.crypto.provider.GaloisCounterMode::checkInit@60 (line 329)
>  ;; B6: #       out( B242 B7 ) <- in( B192 B5 )  Freq: 0.999999
>   0x00000001105665b8:   cbnz    x21, 0x00000001105665c8
>  ;; null oop passed to encode_heap_oop_not_null2
>   0x00000001105665bc:   dcps1   #0xdeae
>   0x00000001105665c0:   .inst   0x082adca6 ; undefined
>   0x00000001105665c4:   udf     #1
>   0x00000001105665c8:   lsr     x10, x21, #3                ;*invokevirtual 
> getLongUnaligned {reexecute=0 rethrow=0 return_oop=0}
>                                                             ; - 
> java.lang.invoke.VarHandleByteArrayAsLongs$ArrayHandle::get@32 (line 115)
>    ... lots more
> 
> 
> but the llvm-based one stops here:
> 
> 
>   0x000000010ed79034:           stp     wzr, wzr, [x20, #0x3c];*putfield 
> preCounterBlock {reexecute=0 rethrow=0 return_oop=0}
>                                                             ; - 
> com.sun.crypto.provider.GaloisCounterMode$GCMEngine::<init>@80 (line 673)
>                                                             ; - 
> com.sun.crypto.provider.GaloisCounterMode$GCMDecrypt::<init>@8 (line 1346)
>                                                             ; - 
> com.sun.crypto.provider.GaloisCounterMode::checkInit@60 (line 329)
>  ;; B6: #       out( B242 B7 ) <- in( B192 B5 )  Freq: 0.999999
>   0x000000010ed79038:           cbnz    x21, #0x10
>  ;; null oop passed to encode_heap_oop_not_null2
>   0x000000010ed7903c:           dcps1   #0xdeae
>   0x000000010ed79040:   
> --------------------------------------------------------------------------------
> [/Disassembly]
> 
> 
> 
> I think it's giving up as soon as it sees something it doesn't recognize, so 
> it's pretty much useless.
> 
> In addition, even when it does work the LLVM disassembly is pretty poor. For 
> example, the unverified entry point looks like
> 
> 
>   0x000000010ed78f80:           ldr     w8, [x1, #0x8]
>   0x000000010ed78f84:           cmp     w9, w8
>   0x000000010ed78f88:           b.eq    #0x10
>   0x000000010ed78f8c:           adrp    x8, #-128524288     ;   {runtime_call 
> ic_miss_stub}
>   0x000000010ed78f90:           add     x8, x8, #0x1c0
>   0x000000010ed78f94:           br      x8
> 
> 
> instead of
> 
> 
>   0x0000000110566500:   ldr     w8, [x1, #8]
>   0x0000000110566504:   cmp     w9, w8
>   0x0000000110566508:   b.eq    0x0000000110566518  // b.none
>   0x000000011056650c:   adrp    x8, 0x0000000108ae6000      ;   {runtime_call 
> ic_miss_stub}
>   0x0000000110566510:   add     x8, x8, #0x1c0
>   0x0000000110566514:   br      x8
> 
> 
> Sure, it's good to have a choice, but the LLVM-based hsdis doesn't look to me 
> like a serious alternative for professional work.

@theRealAph Yes, the build instructions needs to be updated. I could have sworn 
I did that, but maybe that fix is just lying around un-committed in one of my 
repos on one of my test computers. I'll have to go around and check. Or write 
it again if I can't fix it. That needs to go with this PR.

You can do `make install-hsdis` to avoid doing the symlinking yourself. As Jorn 
pointed out, there is currently a bug in that the library does not get copied 
from the "exploded jdk" to the image. I will fix that in an upcoming commit to 
this PR.

As for LLVM not giving you a good user experience; I can't really tell what's 
wrong. Apparently @luhenry (and @JornVernee I believe) has used this. I'm not 
really the target audience myself; I'm only trying to make it possible to use. 
If it is so severly limited as you say maybe this isn't worth pursuing. Some 
feedback from those who have tested it would be appeciated here, to help med 
understand if this patch should be dropped.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5920

Reply via email to