RFR: 8253757: Add LLVM-based backend for hsdis

[Magnus Ihse Bursie]

On Thu, 8 Oct 2020 20:40:50 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> @navyxliu
>> 
>>> @luhenry I tried to build it with LLVM10.0.1
>>> on my x86_64, ubuntu, I ran into a small problem. here is how I build.
>>> $make ARCH=amd64 CC=/opt/llvm/bin/clang CXX=/opt/llvm/bin/clang++ LLVM=/opt/llvm/
>>> 
>>> I can't meet this condition because Makefile defines LIBOS_linux.
>>> 
>>> #elif defined(LIBOS_Linux) && defined(LIBARCH_amd64)
>>>     return "x86_64-pc-linux-gnu";
>>> 
>>> Actually, Makefile assigns OS to windows/linux/aix/macosx (all lower case)and then
>>> CPPFLAGS += -DLIBOS_$(OS) -DLIBOS="$(OS)" -DLIBARCH_$(LIBARCH) -DLIBARCH="$(LIBARCH)" -DLIB_EXT="$(LIB_EXT)"
>> 
>> Interestingly, I did it this way because on my machine `LIBOS_Linux` would get defined instead of `LIBOS_linux`. I tried on WSL which might explain the difference. Could you please share more details on what environment you are using?
>> 
>>> In hsdis.cpp, native_target_triple needs to match whatever Makefile defined. With that fix, I generate llvm version hsdis-amd64.so and it works flawlessly
>> 
>> I'm not sure I understand what you mean. Are you saying we should define the native target triple based on the variables in the Makefile?
>> 
>> A difficulty I ran into is that there is not always a 1-to-1 mapping between the autoconf/gcc target triple and the LLVM one. For example. you pass `x86_64-gnu-linux` to the OpenJDK's `configure` script, but the equivalent target triple for LLVM is `x86_64-pc-linux-gnu`.
>> 
>> Since my plan isn't to use LLVM as the default for all platforms, and because there aren't that many combinations of target OS/ARCH, I am taking the approach of hardcoding the combinations we care about in `hsdis.cpp`.
>
>> @navyxliu
>> 
>> > @luhenry I tried to build it with LLVM10.0.1
>> > on my x86_64, ubuntu, I ran into a small problem. here is how I build.
>> > $make ARCH=amd64 CC=/opt/llvm/bin/clang CXX=/opt/llvm/bin/clang++ LLVM=/opt/llvm/
>> > I can't meet this condition because Makefile defines LIBOS_linux.
>> > #elif defined(LIBOS_Linux) && defined(LIBARCH_amd64)
>> > return "x86_64-pc-linux-gnu";
>> > Actually, Makefile assigns OS to windows/linux/aix/macosx (all lower case)and then
>> > CPPFLAGS += -DLIBOS_$(OS) -DLIBOS="$(OS)" -DLIBARCH_$(LIBARCH) -DLIBARCH="$(LIBARCH)" -DLIB_EXT="$(LIB_EXT)"
>> 
>> Interestingly, I did it this way because on my machine `LIBOS_Linux` would get defined instead of `LIBOS_linux`. I tried on WSL which might explain the difference. Could you please share more details on what environment you are using?
>> 
> I am using ubuntu 18.04. 
> 
> `OS      = $(shell uname)` does initialize OS=Linux in the first place, but later OS is set to "linux" at line 88 of https://openjdk.github.io/cr/?repo=jdk&pr=392&range=05#new-0
> 
> At line 186,  -DLIBOS_linux -DLIBOS="linux" ... It doesn't match line 564 of https://openjdk.github.io/cr/?repo=jdk&pr=392&range=05#new-2
> 
> in my understanding, C/C++ macros are all case sensitive. I got  #error "unknown platform" because of Linux/linux discrepancy.
> 
>> > In hsdis.cpp, native_target_triple needs to match whatever Makefile defined. With that fix, I generate llvm version hsdis-amd64.so and it works flawlessly
>> 
>> I'm not sure I understand what you mean. Are you saying we should define the native target triple based on the variables in the Makefile?
>> 
>> A difficulty I ran into is that there is not always a 1-to-1 mapping between the autoconf/gcc target triple and the LLVM one. For example. you pass `x86_64-gnu-linux` to the OpenJDK's `configure` script, but the equivalent target triple for LLVM is `x86_64-pc-linux-gnu`.
>> 
>> Since my plan isn't to use LLVM as the default for all platforms, and because there aren't that many combinations of target OS/ARCH, I am taking the approach of hardcoding the combinations we care about in `hsdis.cpp`.

Since I found it close to impossible to review the changes when I could not get a diff with the changes done to hsdis.c/cpp, I created a webrev which shows these changes.  I made this by renaming hsdis.cpp back to hsdis.c, and then webrev could match it up. It is available here:

http://cr.openjdk.java.net/~ihse/hsdis-llvm-backend-diff-webrev/

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

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