This is an archive of the discontinued LLVM Phabricator instance.

[ThinLTO] Move -lto-use-new-pm to llvm-lto2, and change it to -use-new-pm.
ClosedPublic

Authored by timshen on Jun 1 2017, 2:11 PM.

Details

Summary

As we teach Clang to use ThinkLTO + new PM, it's good for the users to
inject through Config, instead of setting a flag in the LTOBackend
library. Move the flag to llvm-lto2.

As it moves to llvm-lto2, a new name -use-new-pm seems simpler and as
clear.

Event Timeline

timshen created this revision.Jun 1 2017, 2:11 PM

(FWIW, I clearly like this patch as I sat with you. Mostly interested in tho folks who have worked on this library confirming they're happy with the API direction.)

davide edited edge metadata.Jun 1 2017, 2:37 PM
davide added a subscriber: pcc.

This is fine by me, in fact, IIRC this was the original approach I proposed, but for some reason we ended up using a cl::opt instead. (@pcc could have ideas on this).
We need to update lld as in all my configs I currently pass -mllvm -use-new-pm everywhere, but that's not a big problem (and given I'm probably the only person on the planet who's using that flag, nobody will be annoyed by the change).
I'll update the linker once this one goes in.

timshen added a reviewer: pcc.Jun 1 2017, 2:41 PM
pcc edited edge metadata.Jun 1 2017, 2:59 PM

I suggested before that this could be a cl::opt because we were previously not exposing the new PM switch to users. On the assumption that exposing it to users is a good idea (and given that clang has apparently been doing so for the last few months), it seems reasonable enough to add it to config.

llvm/include/llvm/LTO/Config.h
50

Add to computeCacheKey.

timshen updated this revision to Diff 101128.Jun 1 2017, 3:36 PM

Add UseNewPM to computeCacheKey.

timshen marked an inline comment as done.Jun 1 2017, 3:36 PM
pcc accepted this revision.Jun 1 2017, 3:51 PM

LGTM

This revision is now accepted and ready to land.Jun 1 2017, 3:51 PM
davide accepted this revision.Jun 1 2017, 3:52 PM

Fine to me if Peter is happy.

This revision was automatically updated to reflect the committed changes.

Thanks for the quick review!