[PATCH] D81743: InlineCostAnnotationPrinterPass - Introducing the pass

[Mircea Trofin via Phabricator via llvm-commits]

mtrofin added a comment.

In D81743#2090561 <https://reviews.llvm.org/D81743#2090561>, @knaumov wrote:

> Changed the creation of InlineParams back to default params and added a "to-do" comment.
>  For the purposes of this pass, the idea of which is to give access for non-debug builds to tests that use debug functionality, the InlineParams can be taken as default. My suggestion is to leave the extension of the pass to parametrization with different InlineParams as a "to-do" for now.


I apologize, maybe it's just me - but could you please elaborate the usage scenario, I don't fully follow. Here's what I understand and don't understand so far: in a non-Debug build (say, Release), a developer would run this pass (how?) and then use the results - but how, if the parameters are different from what clang used?

Otherwise the patch is fine, but I think it'd be valuable to capture the intended usage for maintainability - so others can understand why default params are fine there, for instance (or if not, how to correctly change the code, etc).



================
Comment at: llvm/lib/Analysis/InlineCost.cpp:2525
+  TargetTransformInfo TTI(DL);
+  // TO DO:
+  // Redesign the usage of InlineParams for the purposes of this pass. We can
----------------
nit: I think it's "FIXME: Redesign the usage <...>"


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81743/new/

https://reviews.llvm.org/D81743