On Thu, 8 Oct 2020 20:40:50 GMT, Xin Liu <[email protected]> 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
