This is an archive of the discontinued LLVM Phabricator instance.

[COFF] added support for /lldsavetemps
ClosedPublic

Authored by inglorion on Feb 3 2017, 3:03 PM.

Details

Summary

This adds an option to save temporary files generated during link-time optimization. This can be useful for debugging.

Diff Detail

Repository
rL LLVM

Event Timeline

inglorion created this revision.Feb 3 2017, 3:03 PM
ruiu edited edge metadata.Feb 3 2017, 3:57 PM

Can you add a test?

COFF/Driver.cpp
619–622 ↗(On Diff #87038)

You want to move this code outside of this for loop because this loop is for /opt. Handle this just like /implib.

inglorion added inline comments.Feb 3 2017, 4:04 PM
COFF/Driver.cpp
619–622 ↗(On Diff #87038)

I'll be happy to do that. I actually put it under /opt on purpose so as not to conflict with any other options we might want to add in the future. I suppose the "lld" prefix guards against that well enough.

pcc edited edge metadata.Feb 3 2017, 4:04 PM

Should this also write the object files like the ELF linker does?

inglorion updated this revision to Diff 87525.Feb 7 2017, 2:44 PM
  • Made /lldsavetemps a top-level option instead of nesting it under /opt.
  • Also save object files.
  • Added test.
inglorion updated this revision to Diff 87527.Feb 7 2017, 2:47 PM
inglorion retitled this revision from [COFF] added support for /opt:lldsavetemps to [COFF] added support for /lldsavetemps.

update revision title

ruiu added inline comments.Feb 7 2017, 2:56 PM
COFF/Driver.cpp
625 ↗(On Diff #87527)

Since other chunks of code has comments, add the same comment.

// Handle /lldsavetemps
COFF/LTO.cpp
61 ↗(On Diff #87527)

Do we have error() in COFF? I think you want to use fatal().

inglorion added inline comments.Feb 7 2017, 3:18 PM
COFF/LTO.cpp
61 ↗(On Diff #87527)

We do. I'm also working on a patch to make the error limit configurable.

inglorion updated this revision to Diff 87542.Feb 7 2017, 4:09 PM

added comment

pcc accepted this revision.Feb 7 2017, 4:52 PM

LGTM if Rui's happy with it.

This revision is now accepted and ready to land.Feb 7 2017, 4:52 PM
ruiu accepted this revision.Feb 7 2017, 5:02 PM

LGTM

This revision was automatically updated to reflect the committed changes.