[PATCH] D104102: [FuncSpec] Statistics

[Sjoerd Meijer via Phabricator via llvm-commits]

SjoerdMeijer added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/FunctionSpecialization.cpp:466
       // to worry about the passed value.
       if (!Solver.isBlockExecutable(CS.getParent()))
         continue;
----------------
SjoerdMeijer wrote:
> > Looks like this is not covered by a test. Would be good to have one.
> 
> Will look into that.
Added a test for that in rG29843cbc88f6.


================
Comment at: llvm/test/Transforms/FunctionSpecialization/function-specialization-stats.ll:3
+
+; RUN: opt -stats -function-specialization -deadargelim -inline -S < %s 2>&1 | FileCheck %s
+
----------------
fhahn wrote:
> SjoerdMeijer wrote:
> > fhahn wrote:
> > > do we need to run those unrelated passes?
> > Not necessarily, but it kind of nicely illustrates the possibilities with those "clean up" passes, the things function specialisation enables. 
> > 
> > Let me know what you prefer as I am of course happy to remove them.
> > Not necessarily, but it kind of nicely illustrates the possibilities with those "clean up" passes, the things function specialisation enables. 
> > 
> > Let me know what you prefer as I am of course happy to remove them.
> 
> Usually we try to avoid having tests depend on different passes, to reduce churn when making changes to unrelated passes. While it is nice other stats improve, it’s independent of checking the function specialisation stats.
> 
Ok, sure, understood. This is indeed almost an end-to-end test for function specialisation, and it would be good to have that, but it does not necessarily fit here. Will modify this test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104102