LLVM 7.0.1 compatibility #2976
LLVM 7.0.1 compatibility #2976
Conversation
There is a known bug in LLVM 7 that results in a large number of spurious warnings when JITing: https://bugs.llvm.org/show_bug.cgi?id=39577
The `llvm-config --includedir` result only gives one include dir, but `llvm-config --cflags` sometimes gives multiple dirs, and all are necessary. This happens, for example, when working against an LLVM that was built from source using `cmake`. The new approach uses `llvm-config --cflags`, extracts only the flags that add new include dirs, and replaces the `-I` usage with `-isystem` usage that the original ponyc Makefile approach preferred.
LLVM 7 removed the alignment argument from memcpy and memmove intrinsics.
|
So, when compiling ponyc stdlib tests in release mode, I get an LLVM 7 assert fail related to the actor Main
new create(env: Env) =>
apply(env)
fun apply(env: Env) =>
test[I64](env, (0, true))
test[U64](env, (0, true))
fun test[A: Stringable #read](env: Env, expected: (A, Bool)) =>
busywork[A](env, expected._1)
fun busywork[A: Stringable #read](env: Env, expected: A) =>
"." + "." + "." + "." + "." + "." + "." + "." + "." + "." + "." + "." + "."The assert fail looks like this:
The backtrace (with LLVM 7 debugging symbols included) looks like this: The (lldb) p F->dump()
; Function Attrs: nounwind
define private fastcc nonnull %None* @Main_ref_test_U64_val_o2Wbo(%Main* nocapture readnone dereferenceable(256) %this, %Env* noalias nocapture readonly dereferenceable(64) %env, %t2_U64_val_Bool_val %expected) unnamed_addr #2 !dbg !98 !pony.abi !4 {
entry:
call void @llvm.dbg.value(metadata %Main* %this, metadata !97, metadata !DIExpression()), !dbg !200
call void @llvm.dbg.value(metadata %Env* %env, metadata !107, metadata !DIExpression()), !dbg !201
%0 = extractvalue %t2_U64_val_Bool_val %expected, 0
%1 = tail call fastcc %None* @Main_box_busywork_I64_val_owo(%Main* nonnull %this, %Env* nonnull %env, i64 %0), !dbg !202
ret %None* @None_Inst, !dbg !203
}(lldb) p G->dump()
; Function Attrs: nounwind
define private fastcc nonnull %None* @Main_val_test_I64_val_o2wbo(%Main* nocapture readnone dereferenceable(256) %this, %Env* noalias nocapture readonly dereferenceable(64) %env, %t2_I64_val_Bool_val %expected) unnamed_addr #2 !dbg !204 !pony.abi !4 {
entry:
call void @llvm.dbg.value(metadata %Main* %this, metadata !205, metadata !DIExpression()), !dbg !206
call void @llvm.dbg.value(metadata %Env* %env, metadata !207, metadata !DIExpression()), !dbg !208
%0 = extractvalue %t2_I64_val_Bool_val %expected, 0
%1 = tail call fastcc %None* @Main_box_busywork_I64_val_owo(%Main* nonnull %this, %Env* nonnull %env, i64 %0), !dbg !209
ret %None* @None_Inst, !dbg !210
}Note from looking at the LLVM IR that by this point the It fails when trying to run frame #8: 0x0000000004afcb83 ponyc`(anonymous namespace)::MergeFunctions::mergeTwoFunctions(this=0x0000000006541d90, F=0x0000000006401498, G=0x0000000006408fd8) at MergeFunctions.cpp:773
772 Constant *BitcastF = ConstantExpr::getBitCast(F, G->getType());
-> 773 G->replaceAllUsesWith(BitcastF);The At this point, I'm confused as to how the bitcasting code could have ever been expected to work correctly at all, but it's been around in LLVM for some time, so there must be some other factor at play here. I will continue to investigate this. |
|
Okay, I figured out that this LLVM assert fail only happens when the I'm still not entirely sure how or why this bug happens, but I'm able to prevent it by keeping the Note that the issue can be reproduced without ponyc by obtaining the unoptimized LLVM IR for the above minimal program (by running ponyc in debug mode to get it), then running LLVM's optimizer tool (called # (from within the LLVM 7 build tree where the tools like `opt` have already been built:
bin/opt -mergefunc -mergefunc -S --debug-pass=Executions /path/to/pony-example.llAs of now, stdlib tests compile and run successfully for me in ponyc release mode compilation (optimizations enabled). |
|
@kulibali - when you get a chance, could you look at the appveyor failure? Something in the waf script is amiss, I think. Otherwise, this PR is pretty much working as-is - there was one CircleCI failure on an ARM build, but I've kicked the CI to run again and see if it was spurious. |
|
I neglected to update |
|
Seems to be an additional problem on Windows. For sufficiently complex programs (e.g. the This was theoretically fixed in July (https://reviews.llvm.org/rL337950), but at least one other person has encountered it (https://stackoverflow.com/questions/53811108/kaleidoscope-chapter-8-linker-command-failed-with-exit-code-1143). I will investigate further if I have time. |
|
@kulibali i also gave it a try on windows 10. My ponyc is: I tried to compile: https://github.com/mfelsche/pony-appdirs |
|
You have to run with |
|
@kulibali thank you. I made a silly typo. Here is some info on the headers. It seems it contains some packaged functions (COMDAT stuff) without a symbol (is that legit?): this seems to be the result of optimization, as the debug build does not list these: |
In LLVM 7 it seems that the output of `LLVMGetDefaultTargetTriple()` is no longer normalized. On Win32 it now returns `x86_64-pc-win32`, whereupon the COFF object writer assumes that COMDAT sections don't need global names. This fix makes the target triple `x86_64-pc-win32-msvc` for MSVC builds, and the COFF object writer correctly writes symbol names for the COMDAT sections.
|
@jemc do you think this is ready to merge? |
jemc
added
changelog - added
The add lines are from PR ponylang#824 which were changed in ponylang#2976.
This fixes ponylang#3016 by adding lines back introduced in PR ponylang#824. This code conditionally added llvm include directory only if it wasn't already included. These lines were a small part of PR ponylang#2976 which adds LLVM 7.0.1 compatibility.
This PR is a work in progress to collaborate on LLVM 7.0.1 support.
@kulibali already had some commits on a branch, and I've added some more here as well.
I'll continue working on it, but I wanted to make the combined work public here for collaboration in case @kulibali had any more commits to add here.