This is an archive of the discontinued LLVM Phabricator instance.

Port PlaceSafepoints pass to the new pass manager
ClosedPublic

Authored by jandupej on Oct 18 2022, 5:45 AM.

Details

Summary

This patch ports the PlaceSafepoints pass to the new pass manager as it is used by .NET/Mono. Compatibility with the legacy pass manager is maintained by adding PlaceSafepointsLegacyPass. This pass also depends on PlaceBackedgeSafepointsLegacyPass, which has been kept in the legacy-only variant, since it is apparently used only from PlaceSafepointsPass. It has been renamed, though, to indicate its legacy interface.

Diff Detail

Event Timeline

jandupej created this revision.Oct 18 2022, 5:45 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 18 2022, 5:45 AM
jandupej requested review of this revision.Oct 18 2022, 5:45 AM

looks fine, mostly just some nits

llvm/include/llvm/Transforms/Scalar/PlaceSafepoints.h
60

is this necessary?

74

nit: should have newline at end of file

llvm/lib/Transforms/Scalar/PlaceSafepoints.cpp
101

not sure what this is for, but it's not used elsewhere in LLVM so it probably shouldn't be added here

llvm/test/Transforms/PlaceSafepoints/split-backedge.ll
2

-passes=place-safepoints

speryt added a subscriber: speryt.Feb 8 2023, 11:46 AM
speryt added inline comments.
llvm/lib/Transforms/Scalar/PlaceSafepoints.cpp
101

This is most likely something specific to Visual Studio editor. There is short comment about that in clang/lib/Lex/Pragma.cpp

/// Handle "\#pragma region [...]"
///
/// The syntax is
/// \code
///   #pragma region [optional name]
///   #pragma endregion [optional comment]
/// \endcode
///
/// \note This is
/// <a href="http://msdn.microsoft.com/en-us/library/b6xkz944(v=vs.80).aspx">editor-only</a>
/// pragma, just skipped by compiler.
struct PragmaRegionHandler : public PragmaHandler

So it seems it's safe to drop those.

jandupej updated this revision to Diff 497634.Feb 15 2023, 4:29 AM
aeubanks accepted this revision.Feb 15 2023, 9:40 AM

do you need somebody to land this for you?

llvm/lib/Transforms/Scalar/PlaceSafepoints.cpp
342

this should be updated to not use the legacy pm in a followup change

This revision is now accepted and ready to land.Feb 15 2023, 9:40 AM

do you need somebody to land this for you?

That would be great!

do you need somebody to land this for you?

That would be great!

is there an email you'd like to use for the git commit?

do you need somebody to land this for you?

That would be great!

is there an email you'd like to use for the git commit?

jandupej@microsoft.com

This revision was landed with ongoing or failed builds.Feb 17 2023, 9:18 AM
This revision was automatically updated to reflect the committed changes.