This is an archive of the discontinued LLVM Phabricator instance.

[PATCH] [CodeGen] Insert TargetLibraryInfoWrapperPass before anything else.
ClosedPublic

Authored by • koriakin on Jun 26 2016, 2:16 PM.

Details

Summary

Currently, TargetLibraryInfoWrapperPass is inserted by PMBuilder.
However, some passes are inserted manually before the PMBuilder
ones - if any of them happens to use TargetLibraryInfoWrapperPass,
it'll get a default-constructed one, with an unknown target triple.
This happens to InstrProfiling in D21736, breaking it.

Diff Detail

Repository
rL LLVM

Event Timeline

• koriakin retitled this revision from to [PATCH] [CodeGen] Insert TargetLibraryInfoWrapperPass before anything else..
• koriakin updated this object.
• koriakin added a reviewer: asl.
• koriakin set the repository for this revision to rL LLVM.
• koriakin added a subscriber: cfe-commits.
asl edited edge metadata.Jun 26 2016, 2:29 PM

Hrm? Why I'm set to review this?

asl removed a reviewer: asl.Jun 26 2016, 2:29 PM
dnovillo accepted this revision.Jun 27 2016, 5:26 AM
dnovillo edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jun 27 2016, 5:26 AM
mehdi_amini requested changes to this revision.Jun 28 2016, 8:09 AM
mehdi_amini edited edge metadata.

Missing test.

This revision now requires changes to proceed.Jun 28 2016, 8:09 AM
mehdi_amini added inline comments.Jun 28 2016, 8:10 AM
lib/CodeGen/BackendUtil.cpp
459 ↗(On Diff #61914)

It is not super clean to duplicated this code.
Also the minimum is to document why it is done.

Missing test.

This is tested by D21741 (along with D21736).

• koriakin edited edge metadata.

Added a comment explaining the reason.

mehdi_amini added inline comments.Oct 23 2016, 1:43 PM
lib/CodeGen/BackendUtil.cpp
422 ↗(On Diff #75550)

This seems unnecessary?

• koriakin added inline comments.Oct 25 2016, 3:17 AM
lib/CodeGen/BackendUtil.cpp
422 ↗(On Diff #75550)

This ensures FPM passes get the right TLI as well - since I'm removing the PMBuilder.LIbraryInfo assignment, they would otherwise get the default-constructed ones. Or should I keep the assignment instead, and only special-case MPM?

mehdi_amini accepted this revision.Nov 14 2016, 9:49 PM
mehdi_amini edited edge metadata.

LGTM. Thanks!

lib/CodeGen/BackendUtil.cpp
422 ↗(On Diff #75550)

Oh right, I was looking at the existing code and I missed that you were removing the LibraryInfo initialization.

This revision is now accepted and ready to land.Nov 14 2016, 9:49 PM
This revision was automatically updated to reflect the committed changes.