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

Initial Structure for updating to the NewPassManager #167

Draft
wants to merge 57 commits into
base: main
Choose a base branch
from

Conversation

tpatki
Copy link
Member

@tpatki tpatki commented Sep 19, 2024

WIP.
Fixes #162.
Fixes #176.

  • Add a CMake check to check for Clang 14 and above
  • Rename perfflow_weave.hpp/cpp to perfflow_weave_legacy_pass.hpp/cpp
  • Add an example similar to the InjectFuncPass in the perfflow_weave_new_pass.hpp/cpp
  • Test the simple example
  • Migrate the functionality from the legacy pass to the new pass .If applicable, also migrate common functionality across both passes (parsing, insertBefore/insertAfter) to its own header that can be shared if no other changes are needed.
  • Test on Lassen with clang10 and clang14.05 (legacy and new pass)
  • Split perfflow_weave_common.hpp into .hpp/.cpp
  • Cleanup the modifyAnnotatedFunctions() method, make it easier to read/explain
  • Clean up commented code
  • Test with various clang versions on different systems
  • Format code

Testing:

  • Ruby : clang versions 14
  • Corona: clang versions 14
  • Turing: clang versions 14
  • Turing: clang versions 15
  • Turing: clang versions 17
  • Lassen: clang versions 10
  • Lassen: clang versions 14, -gcc module
  • Lassen: clang versions 16, -gcc module
  • Lassen: clang versions 15, non-gcc module (several C++ errors that I have been unable to resolve. Note: we don't have a clang-<>gcc-<> style module for clang15 on lassen. These have nothing to do with our code, but some incompatibility with libstdc++)
  • Lassen: clang versions 17, non-gcc module (Similar errors to clang15, note that here too we don't have the clang-<>gcc-<> style module for clang17 on lassen.)

Note: Adiak related testing that builds on top of this PR has been moved to a separate branch, see #172.

@tpatki tpatki marked this pull request as draft September 19, 2024 06:56
@tpatki tpatki mentioned this pull request Sep 25, 2024
3 tasks
@tpatki tpatki added this to the Release 0.2.0 milestone Sep 25, 2024
@@ -7,11 +7,15 @@ if("${CMAKE_CXX_COMPILER_ID}" STREQUAL "Clang")
# require at least Clang 9.0
if (CMAKE_CXX_COMPILER_VERSION VERSION_LESS 9.0)
message(FATAL_ERROR "Clang++ version must be at least 9.0!")
elseif (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 11)
elseif (CMAKE_CXX_COMPILER_VERSION VERSION_GREATER 15)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we double check these version comparisons? Is PERFFLOWASPECT_CLANG_15_NEWER supposed to be >=clang15 or >clang15?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

>= from what I recall from my testing (returns true of clang 15). I will test again.

tpatki and others added 28 commits October 10, 2024 12:39
does not include PassManagerBuilder.h header that was removed in clang17
getInt8PtrTy removed in clang18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
2 participants