Page MenuHomePhabricator

Add -debugify-and-strip-all to add debug info before a pass and remove it after
ClosedPublic

Authored by dsanders on Apr 10 2020, 11:04 AM.

Details

Summary

This allows us to test each backend pass under the presence
of debug info using pre-existing tests. The tests should not
fail as a result of this so long as it's true that debug info
does not affect CodeGen.

In practice, a few tests are sensitive to this:

  • Tests that check the pass structure (e.g. O0-pipeline.ll)
  • Tests that check --debug output. Specifically instruction dumps containing MMO's (e.g. prelegalizercombiner-extends.ll)
  • Tests that contain debugify metadata as mir-strip-debug will remove it (e.g. fastisel-debugvalue-undef.ll)
  • Tests with partial debug info (e.g. patchable-function-entry-empty.mir had debug info but no !llvm.dbg.cu)
  • Tests that check optimization remarks overly strictly (e.g. prologue-epilogue-remarks.mir)
  • Tests that would inject the pass in an unsafe region (e.g. seqpairspill.mir would inject between register alloc and virt reg rewriter)

In all cases, the checks can either be updated or
--debugify-and-strip-all-safe=0 can be used to avoid being
affected by something like llvm-lit -Dllc='llc --debugify-and-strip-all-safe'

I tested this without the lost debug locations verifier to
confirm that AArch64 behaviour is unaffected (with the fixes
in this patch) and with it to confirm it finds the problems
without the additional RUN lines we had before.

Depends on D77886, D77887, D77747

Diff Detail

Event Timeline

dsanders created this revision.Apr 10 2020, 11:04 AM
vsk added a comment.Apr 10 2020, 11:18 AM

This is really nice!

llvm/lib/CodeGen/TargetPassConfig.cpp
818

Period after 're-run'?

827

I'm confused by this: the comment says that "GlobalISel with the fallback path disabled and -run-pass seem to be unaffected". But here, IIUC if the fallback path is disabled (!isGlobalISelAbortEnabled), debugify is not safe. Is this a contradiction, or have I misunderstood what isGlobalISelAbortEnabled means?

dsanders marked an inline comment as done.Apr 10 2020, 11:27 AM
dsanders added inline comments.
llvm/lib/CodeGen/TargetPassConfig.cpp
827

IIUC if the fallback path is disabled (!isGlobalISelAbortEnabled)

It's a bit unfortunate that we usually talk about the fallback path being enabled/disabled but the code says whether aborting is enabled. !isGlobalISelAbortEnabled means that aborting is disabled so we can continue into the fallback path

vsk accepted this revision.Apr 10 2020, 11:39 AM

Lgtm, thanks!

llvm/lib/CodeGen/TargetPassConfig.cpp
827

Got it.

This revision is now accepted and ready to land.Apr 10 2020, 11:39 AM
dsanders marked 3 inline comments as done.Apr 10 2020, 4:38 PM
This revision was automatically updated to reflect the committed changes.