Skip to content
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

RFC: Deprecate support for LLVM <= 5 #471

Closed
6 tasks done
elliottslaughter opened this issue Dec 9, 2020 · 9 comments
Closed
6 tasks done

RFC: Deprecate support for LLVM <= 5 #471

elliottslaughter opened this issue Dec 9, 2020 · 9 comments
Assignees

Comments

@elliottslaughter
Copy link
Member

elliottslaughter commented Dec 9, 2020

Update: we are in phase 4 below. LLVM <= 5 has been formally deprecated and is planned for removal in late 2022. Please comment on this issue if you are still using LLVM 3.8 in particular (as this is the last release to support NVVM, prior to our switch to the NVPTX backend with LLVM 5+). Also, if you have not had an opportunity to directly compare LLVM 3.8 and newer versions, please take the time to do so.

I'm going to kickstart this discussion early since I know there are still users on LLVM 3.8. I would ideally like to move our minimum LLVM version to 6, i.e. deprecating all versions <= 5.

This will be similar to #456, except that (since I know there are live users on 3.8) I want to make sure we confirm all those users have migrated before we pull the plug.

Steps:

  1. [DONE] Verify all users have moved to at least LLVM 6 and there are no outstanding issues associated with those newer versions.
  2. [DONE] Wait a couple of months to be sure we're not missing anything.
  3. [DONE] Formally deprecate LLVM <= 5. This means changing the README and release notes. No code changes.
  4. [DONE] Wait an additional 6 months before removing the code from the repository.

If you are actively using any LLVM versions <= 5, please reply to this issue as soon as possible so that your usage can be tracked.

Blockers:

@elliottslaughter elliottslaughter self-assigned this Dec 9, 2020
@mariodirenzo
Copy link

We have issues with LLVM 6 on Yellowstone at Stanford. However LLVM 3.8 works just fine.
Maybe @cmelone can comment on this

@cmelone
Copy link

cmelone commented Dec 9, 2020

The solver builds successfully on LLVM 6, but when we run test cases, this error occurs:

Edit: the error report has been moved to #472

I'm not sure how related this is directly to terra, but this is an issue we encounter on 6.0 and not 3.8

@elliottslaughter
Copy link
Member Author

@cmelone @mariodirenzo Thanks for reporting. I'm splitting this off into a separate issue so that we can track it. I'll mark it as a blocker up above.

@elliottslaughter
Copy link
Member Author

There is one large caveat for CUDA users: Terra switched to NVPTX starting with LLVM 5, and that means that certain details of the backends, like what intrinsics are available, differ. See StanfordLegion/legion#983 an example of the sort of issues you might face.

I think think means we'll want to be extra sure we compare performance and compatibility with users of CUDA in Terra.

@elliottslaughter
Copy link
Member Author

CC'ing more people who might have a stake in the CUDA backend: @jameshegarty @gilbo @Mx7f @ProfFan @aiverson @ErikMcClure

Please see above discussion, particularly about the migration from NVVM to NVPTX, and let us know (a) if this impacts you, and (b) what if any testing you would like to do before LLVM 3.8 (and therefore NVVM support) is deprecated.

@elliottslaughter
Copy link
Member Author

Now that I've replicated LLVM 3.8 performance on the apps I've been testing, I am formally kicking off the deprecation period for LLVM <= 5. This puts us in phase 1 above. I have a couple of known users on LLVM 3.8 (via Regent) who I will contact. If anyone else is still using any LLVM version less than or equal to 5, speak up now. This phase will be relatively short (assuming the known users confirm they can move to newer LLVM versions), and then we'll be on to phase 2.

Note that if you're upgrading, there are some things that have changed recently that will (hopefully) improve your performance, especially on NVIDIA GPUs. Some of these things are automatic, but some may require changes on your end. Specifically:

  • The LLVM inliner pass was previously disabled for NVPTX with LLVM 5 and newer. This has been fixed. Note that this may or may not impact performance as PTX has its own inliner.
  • The atomic intrinsics in NVVM were never supported by NVPTX (and therefore, Terra with LLVM 5 or newer). Users were either forced to stick with LLVM 3.8 or switch to inline assembly. You would know if this is you (assuming you've attempted the upgrade). Terra now directly supports the atomicrmw instruction for use with newer LLVM versions. (See here and scroll down to atomicrmw.)
  • NVVM used the equivalent of LLVM's contract fast-math option by default. NVPTX does not. Terra now has explicit support for passing an "optimization profile" to terralib.saveobj and cudalib.toptx, which can include fast-math flags. To achieve the equivalent of the old NVVM performance, pass {fastmath="contract"}. You might see additional performance gains by passing {fastmath=true} to enable all optimizations. Note that in general fast-math optimizations can introduce numerical inaccuracy and may not be safe with every application.

I'm CCing a list of people who have been interested in CUDA support at some point in the past. If you are still using LLVM 3.8, or have not done a direct comparison to newer versions to verify the performance, now is the time to do so. @jameshegarty @gilbo @Mx7f @ProfFan @aiverson @ErikMcClure

@elliottslaughter
Copy link
Member Author

I haven't heard anything, and all the Regent users I've talked to have had performance that is at least as good (and other issues have been resolved), so I'm going to kick off the next phase.

I might accelerate this a bit so we get the deprecation in before 1.0.0, but otherwise things will be as before: we'll wait a bit longer to see if there's any reason not to deprecate, then formalize the deprecation. Nothing will be removed at this point (and not before 1.0.0 at any rate).

@elliottslaughter
Copy link
Member Author

I'm going to pull the trigger for this. Formal deprecation is in #554.

@elliottslaughter elliottslaughter unpinned this issue May 20, 2022
@elliottslaughter
Copy link
Member Author

Final removal was in #612.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants