-
Notifications
You must be signed in to change notification settings - Fork 23
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Versions 6 and 7 progress thread #82
Comments
A minor point: tapir-llvm:release_60 is based on a release candidate of LLVM 6, whereas tapir-llvm:release_60-release is based on the actual LLVM 6.0.0 release. (Rebasing onto a release candidate was my mistake. Lesson learned; in the future, we'll avoid making public branches that are rebased on release candidates.) There are several changes in tapir-llvm:release_60-release having to do with exceptions that you'll want to incorporate. The LLVM-5-era design for how Tapir integrates with exception-handling code is fundamentally wrong, which was the cause of issue #32. The changes in tapir-llvm:release_60 and tapir-llvm:release_60-release fix these problems by adding an optional "unwind" destination to detaches and introducing the "detached-rethrow" intrinsic. These changes affect a fair amount of the codebase, but I believe they are important changes, especially for dealing with C++ code. I would highly encourage incorporating these changes. Although it's not as crucial, you might want to incorporate the TapirTaskInfo pass from the WIP-taskinfo branch as well. These changes deal make it easier for code elsewhere in the codebase to handle Tapir. For example, TapirTaskInfo encapsulates a lot of the ugly code that is needed for dealing with exceptions. I have a lot of confidence in these changes now, and I was planning to move them into tapir-llvm:release_60-release in the near future. One last tip, if you want to find more bug fixes to incorporate, check out the Tapir regression tests in tapir-llvm:release_60 or tapir-llvm:release_60-release. |
Thanks TB. I'm working on cherry-picking those changes. One other set of changes I noticed in release_60 were some changes that added cilk sanitizer and stealable intrinsics. Should those be pulled in as well? |
You mean "cilk sanitizer" and "stealable" function attributes? I think you'll want to pull those in as well, though I don't think they should be too much trouble. We use the stealable attribute to fix some machine-code-generation issues for parallel programs. For Cilk codes, the work-stealing scheduler means that certain optimizations on x86 machine code are not legal to do on functions that might perform a My memory is fuzzy re the cilk-sanitizer attribute, but I think we use that attribute to make sure Cilksan won't ever instrument the same function twice. It's also handy for debugging Cilksan and making sure it instruments all relevant functions, as I recall. |
Whoops, yes I meant the function attributes. Good to know, I'll include them as well. Thanks. |
Also if possible we want to merge in the commits from the gpu WIP codebase as well. @stelleg can you push the branches to the main repo for the sake of testing CI (among other things)? |
I thought I'd open an issue to discuss progress towards up to date versions 6 and 7. stelleg:release_60 is current master rebased on llvm:release_60 that is failing 5 tapir-specific tests, 2 loop fusion and 3 parallel-for loop vectorization tests. Once I've figured those out, I'll make a PR, and start rebasing that branch on llvm:release_70. I used some of the work from tapir-llvm:release_60 and master-v6 to help fix some of the issues rebasing on llvm:release_60. tapir-llvm:release_60 seems to have diverged slightly, making rebasing off of it more difficult than llvm:release_60. Let me know if you want any logic from tapir-llvm:release_60 incorporated before I make the PR.
The text was updated successfully, but these errors were encountered: