This is an archive of the discontinued LLVM Phabricator instance.

[TargetTransformInfo, API] Add optional list of operands to TTI::getUserCost
ClosedPublic

Authored by eastig on Jun 9 2017, 9:29 AM.

Details

Summary

The changes are a result of discussion of D33685.
It solves the following problem:

  1. We can inform getGEPCost about simplified indices to help it with calculating the cost. But getGEPCost does not take into account the context which GEPs are used in.
  2. We have getUserCost which can take the context into account but we cannot inform about simplified indices.

With the changes getUserCost will have access to additional information as getGEPCost has.
The provided list is empty by default.

Event Timeline

eastig created this revision.Jun 9 2017, 9:29 AM
efriedma added inline comments.Jun 14 2017, 3:55 PM
include/llvm/Analysis/TargetTransformInfo.h
228

Not sure I like the way this default argument works... too easy to mess up the "if (Operands.empty())" check. Maybe add a one-argument overload that builds the list and calls the two-argument version?

eastig updated this revision to Diff 102824.Jun 16 2017, 7:29 AM

Added a helper one-argument function which calls the two-argument function with
an empty Operands list.

eastig marked an inline comment as done.Jun 16 2017, 7:31 AM
efriedma edited edge metadata.Jun 16 2017, 12:38 PM

Umm, that misses the point of my comment; we want to reduce the number of paths through the code by requiring that Operands is never empty (except for operations which don't have any operands).

eastig updated this revision to Diff 103276.Jun 20 2017, 3:19 PM

Aha, now I see what you mean. This is an attempt to implement your idea.

Yes, this is what I was meant; thanks.

include/llvm/Analysis/TargetTransformInfo.h
234

std::move doesn't do anything here; ArrayRef is POD.

eastig updated this revision to Diff 103331.Jun 21 2017, 2:40 AM

Removed 'std::move'

eastig marked an inline comment as done.Jun 21 2017, 2:41 AM
efriedma accepted this revision.Jun 28 2017, 4:57 PM

LGTM.

This revision is now accepted and ready to land.Jun 28 2017, 4:57 PM
This revision was automatically updated to reflect the committed changes.
eastig added a comment.Jul 3 2017, 3:06 AM

Thank you, Eli.