LLVM 9.0.x support #3320

chalcolith · 2019-10-04T18:23:27Z

This PR is for compiling with LLVM 9.0.0.

chalcolith · 2019-10-04T18:29:09Z

Ok, will do.

SeanTAllen · 2019-10-04T18:39:26Z

@kulibali i deleted my comment. decided that it was out of scope and should be done as part of a different PR. only thing i think we need is a changelog for 9.0.0 support when it comes.

mshockwave · 2019-10-19T22:50:18Z

I'm wondering if there is any list of pending tasks or known bugs on moving toward 9.0?
The only TODO list I found now are the failing tests in some of the build bots.

chalcolith · 2019-10-20T01:06:43Z

There are failing tests on both Windows and Linux that need to be fixed -- that's the main area that we need help in.

Also the Linux CI setup needs to be updated to build with LLVM 9, but the branch should be rebased before doing that since Sean has been making lots of changes to the CI.

mshockwave · 2019-10-28T23:05:41Z

I just came up with some small fixes, what is the best way to present them here? Fork and PR maybe?

chalcolith · 2019-10-28T23:09:05Z

Yes please.

chalcolith · 2019-10-28T23:10:05Z

Or if the diff is small enough, just include it here.

mshockwave · 2019-10-28T23:12:17Z

The diff is pretty small so I'll just post it here:

diff --git a/src/libponyc/codegen/codegen.c b/src/libponyc/codegen/codegen.c
index 91bf4b3c..1b89b16d 100644
--- a/src/libponyc/codegen/codegen.c
+++ b/src/libponyc/codegen/codegen.c
@@ -1179,7 +1179,7 @@ void codegen_poptry(compile_t* c)

 void codegen_debugloc(compile_t* c, ast_t* ast)
 {
-  if(ast != NULL)
+  if(ast != NULL && c->frame->di_scope != NULL)
   {
 #if PONY_LLVM < 900
     LLVMSetCurrentDebugLocation2(c->builder,
@@ -1194,9 +1194,7 @@ void codegen_debugloc(compile_t* c, ast_t* ast)
 #if PONY_LLVM < 900
     LLVMSetCurrentDebugLocation2(c->builder, 0, 0, NULL);
 #else
-    LLVMMetadataRef loc = LLVMDIBuilderCreateDebugLocation(c->context, 0, 0,
-      c->frame->di_scope, NULL);
-    LLVMSetCurrentDebugLocation2(c->builder, loc);
+    LLVMSetCurrentDebugLocation2(c->builder, NULL);
 #endif
   }
 }

mshockwave · 2019-10-28T23:23:21Z

Here is the root cause:
Previously we use our own wrapper function, LLVMSetCurrentDebugLocation2 (it has the same name as another API in LLVM 9, unfortunately) defined in src/libponyc/codegen/gendebug.cc, to attach the debug MD. That wrapper called DebugLoc::get(...), which will do sanity check on the scope parameter and use an empty DebugLoc (which will not even attached on an instruction actually) instead if the scope is null.

The new API we use, LLVMDIBuilderCreateDebugLocation and LLVMSetCurrentDebugLocation2 in LLVM 9, will not do any sanity check on scope. So it's possible to have a DILocation with a null scope. However, LLVM verify rules doesn't allow null scope, so it bail out.

The fix I just presented will match the behavior of old LLVM: If the scope passed in is null, don't even create a DebugLoc for it.

chalcolith · 2019-10-28T23:49:33Z

Awesome, thanks @mshockwave!

chalcolith · 2019-10-29T15:18:35Z

Windows is working now. The one remaining issue is that CodegenTest.TryBlockCantCatchCppExcept fails on Unix, only when it is run in combination with other codegen tests. If it is run by itself it succeeds.

mshockwave · 2019-10-30T00:05:44Z

Windows is working now. The one remaining issue is that CodegenTest.TryBlockCantCatchCppExcept fails on Unix, only when it is run in combination with other codegen tests. If it is run by itself it succeeds.

Yes I also found that problem. It didn't appear when using LLVM 7 and it's more difficult to debug since most of the stack frames are corrupted when looking into it with a debugger. My rough guess will be something to do with different EH behaviors in LLVM 9 or libunwind.

lenary · 2019-11-18T11:33:31Z

FYI, 9.0.1-final is scheduled for 9 Dec 2019

chalcolith · 2019-12-04T17:47:22Z

Building with 9.0.1-rc1 shows the same problem in CodegenTest.TryBlockCantCatchCppExcept on Linux.

SeanTAllen · 2019-12-14T16:02:53Z

@kulibali all the tests are passing, what environment are you getting a failure in?

SeanTAllen · 2019-12-14T16:04:54Z

O I C, this isn't testing with using 9.0.0, right?

mshockwave · 2019-12-15T01:08:12Z

@SeanTAllen I don't think any of the bot is using 9.x now

mshockwave · 2019-12-15T05:06:11Z

So I finally figure out the problem on the CodegenTest.TryBlockCantCatchCppExcept test case. It took me so long (at least a month I would say) to dig out the root cause, which is pretty complicate and required an separate matter to describe it. So I will only put the solution here:

Again, the problem falls inside the LLVM part. Apply the following patch on the release/9.x branch of LLVM and you're good to go.

diff --git a/llvm/lib/ExecutionEngine/SectionMemoryManager.cpp b/llvm/lib/ExecutionEngine/SectionMemoryManager.cpp
index 925049b2a1b..b30628a6448 100644
--- a/llvm/lib/ExecutionEngine/SectionMemoryManager.cpp
+++ b/llvm/lib/ExecutionEngine/SectionMemoryManager.cpp
@@ -226,6 +226,7 @@ void SectionMemoryManager::invalidateInstructionCache() {
 }

 SectionMemoryManager::~SectionMemoryManager() {
+  deregisterEHFrames();
   for (MemoryGroup *Group : {&CodeMem, &RWDataMem, &RODataMem}) {
     for (sys::MemoryBlock &Block : Group->AllocatedMem)
       MMapper.releaseMappedMemory(Block);

Just an off-topic story: this bug has been discovered since 2015: https://bugs.llvm.org/show_bug.cgi?id=23991
But it never got fixed due to the rapid-changing architecture of ORC JIT in recent years.

Also, this bug is actually pretty hard to triggered in real-world Pony: you need to invoke PonyJIT several times (at lease 10 times) within single program execution.

chalcolith · 2019-12-15T05:15:44Z

Awesome! Thanks so much @mshockwave!

mshockwave · 2019-12-15T07:05:35Z

Update: During my development I temporarily comment out the -static-libstdc++ flag in Makefile:

diff --git a/Makefile-ponyc b/Makefile-ponyc
index 9f031a2d..85875f0c 100644
--- a/Makefile-ponyc
+++ b/Makefile-ponyc
@@ -187,7 +187,7 @@ endif
 ifneq (,$(filter $(link),static llvm-static))
   LLVM_LINKING =--link-static
   ifeq (,$(filter $(OSTYPE),bsd osx))
-    LINKER_FLAGS += -static-libstdc++
+    #LINKER_FLAGS += -static-libstdc++
   endif
   ifneq (,$(shell $(CC) -v 2>&1 | grep gcc))
     LINKER_FLAGS += -static-libgcc

And I found my solution doesn't work if I restore the Makefile 😢
Looks like we need more investigation

chalcolith · 2019-12-17T20:00:52Z

We have decided to:

Disable this test for now, as it seems to be related to the JIT and does not necessarily affect regular programs.
Update vendored LLVM to 9.0.x and merge.
Create issues to investigate further, both the JIT issue and also exception handling on Windows.

dipinhora · 2019-12-18T03:52:54Z

@mshockwave your fix worked for me in a fresh docker container (i confirmed the segfault prior to applying your diff):

[----------] Global test environment tear-down
[==========] 1364 tests from 48 test cases ran. (69793 ms total)
[  PASSED  ] 1364 tests.
root@12d2f5497507:/home/pony/ponyc# ./build/release/ponyc --version
0.33.0-cd7805ad [release]
compiled with: llvm 9.0.1 -- cc (Ubuntu 7.4.0-1ubuntu1~18.04.1) 7.4.0
Defaults: pic=true
root@12d2f5497507:/home/pony/ponyc# git diff
diff --git a/lib/llvm/llvm-8.x.cfg b/lib/llvm/llvm-8.x.cfg
index e73df4e6..d70d3eea 100644
--- a/lib/llvm/llvm-8.x.cfg
+++ b/lib/llvm/llvm-8.x.cfg
@@ -1 +1 @@
-LLVM_CHECKOUT_REF := release/8.x
+LLVM_CHECKOUT_REF := release/9.x
diff --git a/lib/llvm/src b/lib/llvm/src
index 4856a933..c1a0a213 160000
--- a/lib/llvm/src
+++ b/lib/llvm/src
@@ -1 +1 @@
-Subproject commit 4856a9330ee01d30e9e11b6c2f991662b4c04b07
+Subproject commit c1a0a213378a458fbea1a5c77b315c7dce08fd05-dirty
root@12d2f5497507:/home/pony/ponyc# cd lib/llvm/src
root@12d2f5497507:/home/pony/ponyc/lib/llvm/src# git diff
diff --git a/llvm/lib/ExecutionEngine/SectionMemoryManager.cpp b/llvm/lib/ExecutionEngine/SectionMemoryManager.cpp
index 925049b2a1b..b30628a6448 100644
--- a/llvm/lib/ExecutionEngine/SectionMemoryManager.cpp
+++ b/llvm/lib/ExecutionEngine/SectionMemoryManager.cpp
@@ -226,6 +226,7 @@ void SectionMemoryManager::invalidateInstructionCache() {
 }

 SectionMemoryManager::~SectionMemoryManager() {
+  deregisterEHFrames();
   for (MemoryGroup *Group : {&CodeMem, &RWDataMem, &RODataMem}) {
     for (sys::MemoryBlock &Block : Group->AllocatedMem)
       MMapper.releaseMappedMemory(Block);
root@12d2f5497507:/home/pony/ponyc/lib/llvm/src#

mshockwave · 2019-12-18T13:08:21Z

@kulibali that sounds a reasonable decision. This issue has taken me and rest of us too much time. Regardless, I think we still can create a JIT fix patch mentioned earlier for our vendored LLVM

@dipinhora thanks for your verifying. That reminded me a config that was different from the usual building process: I'm linking all the LLVM libraries dynamically.

I will also report our discovery regarding the JIT issue to the LLVM community.

mshockwave · 2019-12-21T08:56:32Z

There was a small twist regarding the JIT fix patch I posted earlier: One of the ORC JIT creators, Lang Hames, also committed a patch to solve the same problem (i.e. missing EH frames deregistration in ORCv2) but with a more general solution. So I think (before we synchronize with upstream LLVM) we should use that patch instead.

Also, I did some extra experiments to confirm that under the following conditions, both my patch and Lang's patch will fix our problem:

Link libstdc++ statically
Link LLVM libraries statically
Link libgcc dynamically

Which, are the "normal" build configurations.

@mshockwave

Thanks very much to @mshockwave for their help!

SeanTAllen · 2019-12-23T21:50:06Z

@kulibali LLVM 9.0.1 doesn't seem to have been released yet, what version is the vendored LLVM updated to? 9.0.0? an rc candidate for 9.0.1?

chalcolith · 2019-12-23T21:52:48Z

9.0.1 final has been tagged; the vendored LLVM is at c1a0a213378a458fbea1a5c77b315c7dce08fd05 llvmorg-9.0.1.

SeanTAllen · 2019-12-23T22:25:41Z

Awesome. @kulibali I'm going to add a couple of CHANGELOG entries to this PR.

SeanTAllen · 2019-12-23T22:35:16Z

@kulibali I fixed a small formatting error. I'm going to make sure it passes CI and will then merge.

SeanTAllen · 2019-12-23T23:09:33Z

Party time @kulibali. This one was merged.

chalcolith added do not merge This PR should not be merged at this time LLVM labels Oct 4, 2019

chalcolith mentioned this pull request Oct 4, 2019

WIP compiling with LLVM 8.0.0 #3034

Closed

chalcolith mentioned this pull request Oct 16, 2019

Compiler crash in ponyc 0.28.0 with LLVM 7+ for certain projects #3140

Closed

chalcolith force-pushed the llvm900 branch from c0a4688 to cd7805a Compare December 3, 2019 22:21

Update to allow building with LLVM 9.0.0.

0d13a89

Thanks very much to @mshockwave for their help!

chalcolith mentioned this pull request Dec 23, 2019

Patch LLVM and re-enable unit test CodegenTest.TryBlockCantCatchCppExcept for LLVM >= 9.0.0 #3434

Closed

chalcolith added 3 commits December 23, 2019 10:56

Update supported version of VS2019 on Windows.

8f758d7

Disable CodegenTest.TryBlockCantCatchCppExcept for LLVM >= 9.0.0

79ddcbb

Update vendored LLVM to 9.0.1

68f5f6b

chalcolith force-pushed the llvm900 branch from cd7805a to 68f5f6b Compare December 23, 2019 19:14

chalcolith removed the do not merge This PR should not be merged at this time label Dec 23, 2019

chalcolith marked this pull request as ready for review December 23, 2019 19:15

chalcolith changed the title ~~WIP compiling LLVM 9.0.0~~ Compiling with LLVM 9.0.1 Dec 23, 2019

chalcolith requested a review from SeanTAllen December 23, 2019 21:05

SeanTAllen changed the title ~~Compiling with LLVM 9.0.1~~ LLVM 9.0.x support Dec 23, 2019

SeanTAllen added the changelog - added Automatically add "Added" CHANGELOG entry on merge label Dec 23, 2019

Clean up small formatting issue

d7022a0

SeanTAllen approved these changes Dec 23, 2019

View reviewed changes

SeanTAllen merged commit 0fea9f8 into master Dec 23, 2019

SeanTAllen deleted the llvm900 branch December 23, 2019 23:09

github-actions bot pushed a commit that referenced this pull request Dec 23, 2019

Update CHANGELOG for PR #3320 [skip ci]

ab124be

SeanTAllen mentioned this pull request Dec 23, 2019

Last Week in Pony - December 29, 2019 ponylang/ponylang-website#527

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LLVM 9.0.x support #3320

LLVM 9.0.x support #3320

chalcolith commented Oct 4, 2019

chalcolith commented Oct 4, 2019

SeanTAllen commented Oct 4, 2019

mshockwave commented Oct 19, 2019

chalcolith commented Oct 20, 2019

mshockwave commented Oct 28, 2019

chalcolith commented Oct 28, 2019

chalcolith commented Oct 28, 2019

mshockwave commented Oct 28, 2019

mshockwave commented Oct 28, 2019

chalcolith commented Oct 28, 2019

chalcolith commented Oct 29, 2019

mshockwave commented Oct 30, 2019

lenary commented Nov 18, 2019

chalcolith commented Dec 4, 2019

SeanTAllen commented Dec 14, 2019

SeanTAllen commented Dec 14, 2019

mshockwave commented Dec 15, 2019

mshockwave commented Dec 15, 2019

chalcolith commented Dec 15, 2019

mshockwave commented Dec 15, 2019

chalcolith commented Dec 17, 2019

dipinhora commented Dec 18, 2019

mshockwave commented Dec 18, 2019

mshockwave commented Dec 21, 2019 •

edited

Loading

SeanTAllen commented Dec 23, 2019

chalcolith commented Dec 23, 2019

SeanTAllen commented Dec 23, 2019

SeanTAllen commented Dec 23, 2019

SeanTAllen commented Dec 23, 2019

LLVM 9.0.x support #3320

LLVM 9.0.x support #3320

Conversation

chalcolith commented Oct 4, 2019

chalcolith commented Oct 4, 2019

SeanTAllen commented Oct 4, 2019

mshockwave commented Oct 19, 2019

chalcolith commented Oct 20, 2019

mshockwave commented Oct 28, 2019

chalcolith commented Oct 28, 2019

chalcolith commented Oct 28, 2019

mshockwave commented Oct 28, 2019

mshockwave commented Oct 28, 2019

chalcolith commented Oct 28, 2019

chalcolith commented Oct 29, 2019

mshockwave commented Oct 30, 2019

lenary commented Nov 18, 2019

chalcolith commented Dec 4, 2019

SeanTAllen commented Dec 14, 2019

SeanTAllen commented Dec 14, 2019

mshockwave commented Dec 15, 2019

mshockwave commented Dec 15, 2019

chalcolith commented Dec 15, 2019

mshockwave commented Dec 15, 2019

chalcolith commented Dec 17, 2019

dipinhora commented Dec 18, 2019

mshockwave commented Dec 18, 2019

mshockwave commented Dec 21, 2019 • edited Loading

SeanTAllen commented Dec 23, 2019

chalcolith commented Dec 23, 2019

SeanTAllen commented Dec 23, 2019

SeanTAllen commented Dec 23, 2019

SeanTAllen commented Dec 23, 2019

mshockwave commented Dec 21, 2019 •

edited

Loading