Page MenuHomePhabricator

[InlineCost] Add flag to allow changing the default inline cost
ClosedPublic

Authored by Holman on Wed, Jan 22, 10:32 AM.

Details

Summary

It can be useful to tune the default inline threshold without overriding other inlining thresholds (e.g. in code compiled for size).

The existing -inline-threshold flag overrides other thresholds, so it is insufficient in codebases where there is a mix of code compiled for size and speed.

Diff Detail

Event Timeline

Holman created this revision.Wed, Jan 22, 10:32 AM

@davidxl and @mtrofin for thoughts.

Seems reasonable to decouple the setting of the default from the overriding of the opt size thresholds. However, with your change the only use of InlineThreshold is essentially as a bool flag (was it specified or not). How about leave the InlineThreshold option alone except add a new bool flag to control whether it applies to the opt size thresholds?

Also, needs a test.

This change won't work. See

if (InlineThreshold.getNumOccurrences() == 0) {
  Params.OptMinSizeThreshold = InlineConstants::OptMinSizeThreshold;
  Params.OptSizeThreshold = InlineConstants::OptSizeThreshold;
  Params.ColdThreshold = ColdThreshold;
} else if (ColdThreshold.getNumOccurrences() > 0) {
  Params.ColdThreshold = ColdThreshold;
}

ok. I see the intention. As long as the behavior of --inline-threshold option is not changed (it still overrides the new option), the new option seems fine to me. Adding individually controlled option for size opt seems a good idea too.

there are checks of explicit occurrences of inline-threshold option later, so the behavior seems unchanged.

Holman updated this revision to Diff 241249.Wed, Jan 29, 12:00 PM

Added test

This revision is now accepted and ready to land.Wed, Jan 29, 1:34 PM

Thanks! I don't have commit access, so can someone help me with that?

This revision was automatically updated to reflect the committed changes.