Page MenuHomePhabricator

[CaptureTracking] Make MaxUsesToExplore cheaper (NFC)
ClosedPublic

Authored by nikic on Apr 23 2020, 11:05 AM.

Details

Summary

The change in D78624 had a noticeable negative compile-time impact. It seems that going through a function call for the MaxUsesToExplore default is fairly expensive, at least if LLVM is not built with LTO.

This patch makes MaxUsesToExpore default to 0 and assigns the actual default in the implementation instead. This recovers most of the regression.

Diff Detail

Event Timeline

nikic created this revision.Apr 23 2020, 11:05 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 23 2020, 11:05 AM

Doc for function should probably explicitly mention that zero value means using default option value.

lib/Analysis/CaptureTracking.cpp
233 ↗(On Diff #259634)

Why did you decide to update only one PointerMayBeCaptured function instead of three?

nikic marked an inline comment as done.Apr 23 2020, 12:14 PM
nikic added inline comments.
lib/Analysis/CaptureTracking.cpp
233 ↗(On Diff #259634)

This function is the actual implementation, all other functions just forward to it.

jdoerfert accepted this revision.Apr 23 2020, 12:24 PM

LGTM once you addressed the doc thing mentioned by @skatkov.

This revision is now accepted and ready to land.Apr 23 2020, 12:24 PM
lebedev.ri added inline comments.
include/llvm/Analysis/CaptureTracking.h
40 ↗(On Diff #259634)

Have you considered Optional<unsigned> MaxUsesToExploreOverride
And if it's not provided, actually override it.

skatkov accepted this revision.Apr 23 2020, 7:31 PM

LGTM after fixing the doc.

nikic updated this revision to Diff 259952.Apr 24 2020, 12:13 PM

Add commenting indication that zero = default.

This revision was automatically updated to reflect the committed changes.