This is an archive of the discontinued LLVM Phabricator instance.

[Debugify] Port verify-debuginfo-preserve to NewPM
ClosedPublic

Authored by ntesic on Dec 8 2021, 7:49 AM.

Details

Summary

Debugify in OriginalDebugInfo mode, introduced with D82545 , runs only with legacy PassManager.
This patch enables this utility for the NewPM.

Diff Detail

Event Timeline

ntesic created this revision.Dec 8 2021, 7:49 AM
ntesic requested review of this revision.Dec 8 2021, 7:49 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 8 2021, 7:49 AM
ormris removed a subscriber: ormris.Jan 24 2022, 11:13 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 19 2022, 8:45 AM
Herald added a subscriber: StephenFan. · View Herald Transcript
aeubanks added inline comments.
llvm/lib/Transforms/Utils/Debugify.cpp
964–970

should this and the other addition below be separate passes? doesn't look like the two modes share a lot

llvm/test/DebugInfo/verify-di-preserve.ll
16

I'd rather not test the legacy PM, it's in the process of being removed, ditto for the other RUN lines

llvm/tools/opt/opt.cpp
204

we should move all these options into NewPMDriver.cpp and remove support for them in the legacy PM.
can be a separate change though

Thank you for the comments, @aeubanks.
My replies are inline.

llvm/lib/Transforms/Utils/Debugify.cpp
964–970

As I am aware, this was already discussed before the introduction of the "Original DebugInfo mode" (please, find the link to the discussion) and an agreement was to keep the single pass with two modes.

llvm/test/DebugInfo/verify-di-preserve.ll
16

I agree, now that I am seeing some test platforms don't support '-flegacy-pass-manager', it would be best to test only default (new) PM.

llvm/tools/opt/opt.cpp
204

If you agree, I would keep that as future work, to move those options for both SytheticDebugInfo and OriginalDebugInfo mode together.

ntesic updated this revision to Diff 432865.May 30 2022, 1:09 AM
  • Rebase
  • Don't use legacy Pass Manager in tests
aprantl accepted this revision.Jun 7 2022, 1:47 PM
This revision is now accepted and ready to land.Jun 7 2022, 1:47 PM
This revision was landed with ongoing or failed builds.Jul 6 2022, 8:07 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptJul 6 2022, 8:07 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript