[PATCH] D98439: [NFC][Inliner] Debugging support to print funtion size after each inlining.

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/Inliner.cpp:842
 
+      LLVM_DEBUG(dbgs() << "    Function size: " << F.getInstructionCount()
+                        << "\n");
----------------
wenlei wrote:
> hoy wrote:
> > hoy wrote:
> > > hoy wrote:
> > > > wenlei wrote:
> > > > > We have debug prints for negative decision, but not positive decision, so we may end up with a bunch of "function size: ..". Would it be more helpful to say "Function size: X after inlining Y" or something like that?
> > > > We actually have debug prints for positive decision. With the size printing, the debug log now looks like
> > > > 
> > > > 
> > > > ```
> > > > 
> > > > Inlining calls in: _ZN11opentracing2v24util6detail10dispatcherIRN8facebook7tracing11opentracing12_GLOBAL__N_112ValueVisitorENS0_5ValueENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEJDnPKcNS1_17recursive_wrapperISt6vectorISA_SaISA_EEEENSJ_ISt13unordered_mapISG_SA_St4hashISG_ESt8equal_toISG_ESaISt4pairIKSG_SA_EEEEEEE11apply_constERKSA_S9_
> > > >     Function size: 11
> > > >     Inlining (cost=always): always inline attribute, Call:   %4 = call zeroext i1 @_ZNK11opentracing2v24util7variantIJbdlmNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEDnPKcNS1_17recursive_wrapperISt6vectorINS0_5ValueESaISD_EEEENSB_ISt13unordered_mapIS8_SD_St4hashIS8_ESt8equal_toIS8_ESaISt4pairIKS8_SD_EEEEEEE2isIDnEEbv(%"class.opentracing::v2::util::variant"* nonnull %3), !dbg !35441
> > > >     Function size: 13
> > > >     Inlining (cost=always): always inline attribute, Call:   %8 = call dereferenceable(8) i8** @_ZNK11opentracing2v24util7variantIJbdlmNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEDnPKcNS1_17recursive_wrapperISt6vectorINS0_5ValueESaISD_EEEENSB_ISt13unordered_mapIS8_SD_St4hashIS8_ESt8equal_toIS8_ESaISt4pairIKS8_SD_EEEEEEE3getIDnLPv0EEERKT_v(%"class.opentracing::v2::util::variant"* nonnull %3), !dbg !35458
> > > >     Function size: 26
> > > >     Inlining (cost=-35, threshold=375), Call:   %20 = call dereferenceable(8) i8** @_ZN11opentracing2v24util6detail9unwrapperIDnE11apply_constERKDn(i8** nonnull dereferenceable(8) %19), !dbg !35486
> > > >     Function size: 25
> > > >     Inlining (cost=-15020, threshold=375), Call:   call fastcc void @_ZNK8facebook7tracing11opentracing12_GLOBAL__N_112ValueVisitorclIDnEENSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEET_(%"class.std::__cxx11::basic_string"* noalias %0), !dbg !35501
> > > >     Function size: 31
> > > >     Inlining (cost=-30, threshold=375), Call:   call fastcc void @_ZSt7forwardIRN8facebook7tracing11opentracing12_GLOBAL__N_112ValueVisitorEEOT_RNSt16remove_referenceIS6_E4typeE() #26, !dbg !35617
> > > >     Function size: 30
> > > > ```
> > > > 
> > > Well, I think you are right. I was looking at an older version and it had debug prints for positive decision there.
> > I double checked the latest code base. The inliner advisor actually prints out positive decisions (see `llvm::shouldInline`) and the debug output with this change will look like
> > 
> > 
> > ```
> > Inlining calls in: foo
> >     Function size: 2
> >     Inlining (cost=25, threshold=75), Call:   call void @bar(i1 true)
> >     Function size: 3
> > Updated inlining SCC: (foo)
> > Inlined an internal call edge and split an SCC, retaining this to avoid infinite inlining.
> > Inlining calls in: foo
> >     Function size: 3
> >     NOT Inlining (cost=never): recursive, Call:   call void @foo()
> > Inlining calls in: foo
> >     Function size: 3
> >     NOT Inlining (cost=never): recursive, Call:   call void @foo()
> > Inlining calls in: bar
> >     Function size: 6
> >     NOT Inlining (cost=never): recursive, Call:   call void @foo()
> >     Inlining (cost=25, threshold=75), Call:   call void @bar(i1 true)
> >     Function size: 7
> >     NOT Inlining (cost=never): recursive, Call:   call void @foo()
> > Updated inlining SCC: (bar)
> > Inlining calls in: test1
> >     Function size: 3
> >     Inlining (cost=25, threshold=75), Call:   call void @bar(i1 true)
> >     Function size: 4
> >     Inlining (cost=25, threshold=75), Call:   call void @bar(i1 false)
> >     Function size: 5
> >     NOT Inlining (cost=never): recursive, Call:   call void @foo()
> >     NOT Inlining (cost=never): recursive, Call:   call void @foo()
> > Updated inlining SCC: (test1)
> > Inlining calls in: test2
> >     Function size: 3
> >     Inlining (cost=-30, threshold=75), Call:   call void @noop()
> >     Function size: 2
> >     Inlining (cost=-30, threshold=75), Call:   call void @noop() [ "CUSTOM_OPERAND_BUNDLE"() ]
> > Updated inlining SCC: (test2)
> > Inlining calls in: test3
> >     Function size: 2
> >     NOT Inlining (cost=never): recursive call, Call:   call void @test3() #0
> > ```
> ok, thanks for checking. nit: use "Size after inlining"? From the print out above, currently it looks more like size of the inlinee. 
Sounds good.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D98439