Migrated to LLVM 6.0 #17
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Changed module to qmodule * Added make -DISABLE_STATIC=1 to link with shared libraries * Added CC=clang CXX=clang++ to use LLVM Toolchain instead of GCC to build ScaffCC * Changes made to use LLVM PASS efficiently Reviewers: ah744, Pranav Gokhale, Fred Chong Labels: need review
singular-value
requested changes
Dec 21, 2017
There was a problem hiding this comment.
Hi Leslie, I am just starting to review the code now. Thanks again for doing this.
It looks like the pull request is built on top of the commit from September 6th (c3d97d5), which means that it overwrites/undoes @ajavadia's two commits from November 16-17 (90554eb and 871865b). Would you be able to preserve these two commits, as they fix some bugs?
In the meantime, I'll be reading through the rest of your changes.
-Pranav
|
Hi Pranav, I workaround for But I am not familiar with Quantum Algorithm implemented in scaffold, I am good at C/C++ and Assembly, even that scaffold is C-like language, sorry it is beyond my knowledge, I am only B.S. Computer Science https://www.llvm.org.cn/xiangzhai/ Regards, |
|
Thanks @xiangzhai, okay we will figure out the quantum-related changes later then. I've just gone through some llvm + clang tutorials (I'm new to both) to understand the general changes better. One question I have for you: in the current codebase, there are Scaffold-specific passes in llvm/lib/Transforms/Scaffold. Since you removed the llvm directory, what happens to these passes? |
|
Oh, actually never mind, ignore my previous comment. I understand now. (I had been looking at https://github.com/ScaffCC/ScaffCC where you had separated llvm and clang to https://github.com/ScaffCC/scaff-llvm and https://github.com/ScaffCC/scaff-clang) |
|
@singular-value yes, the advantage of separated llvm and clang is to make repository small, and easy to merge upstream. |
|
@xiangzhai |
|
By the way @xiangzhai, out of curiosity, how did you fix the code formatting/style issues in binary_welded_tree.n100s100.scaffold (and others). Did you use a linter? |
|
Oh I'm guessing you used clang-format? |
|
@singular-value yes clang-format :) http://clang.llvm.org/docs/ClangFormat.html |
|
Thanks :) One more request, could you also incorporate the fixed mac OS X compilation from https://github.com/ScaffCC/ScaffCC/commit/56a8f7fc8de6cfd043df473701df6502246b34da? |
|
Also, I've now finished reading through all of your code. Thanks again for undertaking this effort. However, the regression tests ( I'm ran this on Ubuntu 14.04. Do you know what the issue is? The regression tests passes fine in master. |
|
@singular-value you need to build the whole ScaffCC as below directory tree, if still failed to work, I will check it again :) |
A patch by Eddie Schoute! #17
|
@singular-value you need to build the whole ScaffCC :) the directory tree looks like: Then goto scripts directory, run the regression test: Oops segfault! it might owing to this issue I will git merge from upstream with release_60 branch and check whether or not fix such issue, if not, I will report bug to llvm. |
|
Hmm, this is what I get when I run |
|
I only test it under Linux, and @eddieschoute tested https://github.com/ScaffCC under MacOS, I can't reproduce your issue, but I can still reproduce the segfault issue even merged the latest release_60 branch... I don't want to workaround https://github.com/ScaffCC/scaff-llvm/issues/1 just like comment the |
|
I see, thanks for the update. In the meantime, I'll look into my issue with |
|
@xiangzhai have you had any luck finding the root cause? |
|
@singular-value nope :( I even merged LLVM 7.0 upstream, it is still reproducible, not easy to fix, but workaround :) |
|
I tried compiling your pull request on Ubuntu 16.04 (since it hasn't been working on 14.04, because of the issue I pasted above). Now it is running into a clang++ compilation issue for Rotations/sqct/netgenerator.cpp: So this file compiles with GCC, but not with clang (https://stackoverflow.com/a/41405172). Did you encounter this? |
|
@singular-value it is not reproducible for me: What is your clang's version? perhaps it is better to reimplement the ugly sqct and choose BSD or Apache license :) |
|
I see, yeah clang version number was the issue. I was using 3.8. I upgraded to 3.9 and the file now compiles (also tested with 5.0 and 6.0 and file compiles correctly). So I guess we should change the README again to specify that we need 3.9 or higher (the recent commit specified the minimum clang version as 3.8). I can take care of that. However, although the netgenerator.cpp file now compiles, I now run into the same build issues that I pasted above. Many errors like this: I'm still not sure what this means. |
|
Hi Leslie, Just a couple of updates:
-Pranav |
|
@eddieschoute might be able to help you compiling https://github.com/ScaffCC on MacOS, and I only test it on Fedora 25, the same story is DragonEgg https://github.com/xiangzhai/dragonegg/wiki/Dependence Segfault during regression tests is not easy to fix https://github.com/ScaffCC/scaff-clang/issues/2 Regards, |
|
Hi Leslie, I have an issue regarding building Scaffold, I am using centos7, LLVM/clang version 6.0.1 and I got many warnings and 2 errors: I am new to clang and scaffold so any advice? |
|
@marsscout22 Please try this https://github.com/ScaffCC by following https://github.com/ScaffCC/ScaffCC#installation Regards, |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi ScaffCC developers,
Reviewers: @ah744, Pranav Gokhale, Fred Chong
Labels: need review
Regards,
Leslie Zhai - a LLVM developer https://reviews.llvm.org/p/xiangzhai/