On Mon, 26 Oct 2020 11:16:28 GMT, Magnus Ihse Bursie <i...@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? >>> >> 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/ Some notes (perhaps most to myself) about how this ties into the existing hsdis implementation, and with JDK-8188073 (Capstone porting). When printing disassembly from hotspot, the current solution tries to locate and load the hsdis library, which prints disassembly using bfd. The reason for using the separate library approach is, as far as I can understand, perhaps a mix of both incompatible licensing for bfd, and a wish to not burden the jvm library with additional bloat which is needed only for debugging. The Capstone approach, in the prototype patch presented by Jorn in the issue, is to create a new capstonedis library, and dispatch to it instead of hsdis. The approach used in this patch is to refactor the existing hsdis library into an abstract base class for hsdis backends, with two concrete implementations, one for bfd and one for llvm. Unfortunately, I think the resulting code in hsdis.cpp in this patch is hard to read and understand. ------------- PR: https://git.openjdk.java.net/jdk/pull/392
