This is an archive of the discontinued LLVM Phabricator instance.

Refactor InlinCost.cpp in preparation for estimated speedup heuristic.
AbandonedPublic

Authored by eraman on Feb 16 2017, 5:27 PM.

Details

Reviewers
chandlerc
davidxl
Summary

This includes the following refactoring that is useful for a subsequent patch that adds a new inlining heuristic:

  • Factor out the code in instruction visitors that checks if an instruction evaluates to a constant.
  • Add a method to accumulate the cost of inlining.
  • Make isGEPOffsetConstant (which is called once) into a lambda.
  • Move a block of code in visitSwitchInst to a lambda.

Some of these may not make much sense standalone, but are useful for the subsequent patch (to be posted shortly)

Diff Detail

Event Timeline

eraman created this revision.Feb 16 2017, 5:27 PM
chandlerc edited edge metadata.Feb 16 2017, 5:38 PM

Can you split this into separate patches that do a single refactoring? (Especially considering you have the breakdown in your descirption...) That would make it easier to review for me.

lib/Analysis/InlineCost.cpp
402–404

Of all of these, this one seems the one that doesn't really make sense on its own. I'd rather this be part of the change where it starts to make sense rather than a seemingly bad code smell being added because of some subsequent change.

Can you split this into separate patches that do a single refactoring? (Especially considering you have the breakdown in your descirption...) That would make it easier to review for me.

Ok, will do.

lib/Analysis/InlineCost.cpp
402–404

Ok