[PATCH] D88389: [M68k] (Patch 3/8) Basic infrastructures and target description files

[Jessica Clarke via Phabricator via llvm-commits]

jrtc27 added inline comments.


================
Comment at: llvm/cmake/modules/HandleLLVMOptions.cmake:326-330
+# GCC m68k on Linux by default aligns on 16bit, we want 32
+if(LLVM_INFERRED_HOST_TRIPLE STREQUAL "m68k-unknown-linux-gnu")
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -malign-int")
+    set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -malign-int")
+endif()
----------------
glaubitz wrote:
> jrtc27 wrote:
> > RKSimon wrote:
> > > jrtc27 wrote:
> > > > This is a non-standard ABI and thus is incompatible with system libraries, so IMO is a blocker to merging a native m68k LLVM build. You can still merge cross-compilation support without this though.
> > > I don't think this is a blocker but you should more explicitly mention the potential issue on native in the comment.
> > I do not want this to land. It is broken and should never be used unless you are very very careful. If you want to compile LLVM and take the risk of ABI issues, you can manually add -malign-int to CMAKE_C[XX]_FLAGS when invoking CMake yourself.
> Well, we're still gonna be an experimental backend and it's very important to finally get the status quo upstreamed.
> 
> As there is interest from many retro communities to get this backend into shape, I'm very confident we will get this issue ironed out soonish.
> 
> It's one of the things I was planning to work on over the holidays.
> 
> Please let's don't fail this on the last couple of meters. It's a known and documented issue and we will get around to fixing it.
> 
> I have been waiting for this backend to be merged for ages :(.
If you don't want it to be a blocker, just remove it, and have it documented that if you want to compile LLVM natively you need to pass an extra flag to CMake, with a warning that it may cause hard-to-debug breakage. It's also only necessary for a *native* LLVM, when most people would probably rather cross-compile for m68k than use a slow machine or emulator.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D88389/new/

https://reviews.llvm.org/D88389