This is an archive of the discontinued LLVM Phabricator instance.

[IPSCCP] Create a Pass parameter to control specialization of functions.
ClosedPublic

Authored by labrinea on Dec 22 2022, 9:43 AM.

Details

Summary

Required for D140210 in order to disable FuncSpec at {Os, Oz} optimization levels.

Diff Detail

Event Timeline

labrinea created this revision.Dec 22 2022, 9:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 22 2022, 9:43 AM
labrinea requested review of this revision.Dec 22 2022, 9:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 22 2022, 9:43 AM
Herald added a subscriber: wdng. · View Herald Transcript
chill added inline comments.Dec 23 2022, 2:45 AM
llvm/test/Transforms/FunctionSpecialization/function-specialization.ll
1–2

For completeness, I'd suggest a few more run lines, so ultimately we cover:

PassesOptions
ipsccpnone+
ipsccp-specialize-functions=true
ipsccp-specialize-functions=false
ipsccp<func-spec>none+
ipsccp<func-spec>-specialize-functions=false
ipsccp<no-func-spec>none
ipsccp<no-func-spec>-specialize-functions=true+
nikic added a subscriber: nikic.Dec 23 2022, 3:00 AM
nikic added inline comments.
llvm/test/Transforms/FunctionSpecialization/function-specialization.ll
1–2

Maybe drop -specialize-functions entirely? Is there any benefit to keeping it if we already have the pass parameter?

chill added inline comments.Dec 23 2022, 3:42 AM
llvm/test/Transforms/FunctionSpecialization/function-specialization.ll
1–2

Agree.
When we were discussing it we weren't sure what's the policy and looking at some other passes they have both a pass option and a command line option equivalent, but of course, that may well have been for retaining compatibility (not update thousand tests, etc).

nikic added inline comments.Dec 23 2022, 3:57 AM
llvm/test/Transforms/FunctionSpecialization/function-specialization.ll
1–2

I think this may be a LegacyPM leftover, where pass options didn't exist (or rather, they existed, but not in a form that's directly accessible via opt).

labrinea added inline comments.Dec 23 2022, 4:05 AM
llvm/test/Transforms/FunctionSpecialization/function-specialization.ll
1–2

What happens in case like --passes='default<O3>' -specialize-functions (see compiler-crash-58759.ll) ?

labrinea marked an inline comment as not done.Dec 23 2022, 4:13 AM
labrinea added inline comments.
llvm/test/Transforms/FunctionSpecialization/function-specialization.ll
1–2

Transform to --passes='default<O3>,ipsccp<func-spec>' ?

labrinea updated this revision to Diff 485089.Dec 23 2022, 4:44 AM
labrinea marked an inline comment as not done.
labrinea edited the summary of this revision. (Show Details)
  • Removed the cmdline option -specialize-functions.
  • Changed the IPSCCPOption 'AllowFuncSpec' from std::optional<bool> to bool.
labrinea added inline comments.Dec 23 2022, 4:45 AM
llvm/test/Transforms/FunctionSpecialization/compiler-crash-58759.ll
1

This can be removed in D140210.

labrinea marked 3 inline comments as done.Dec 23 2022, 4:45 AM
chill accepted this revision.Dec 23 2022, 6:58 AM
chill added inline comments.
llvm/include/llvm/Transforms/IPO/SCCP.h
38

Nit: spaces around =

This revision is now accepted and ready to land.Dec 23 2022, 6:58 AM
This revision was landed with ongoing or failed builds.Dec 23 2022, 9:06 AM
This revision was automatically updated to reflect the committed changes.