[PATCH] Consider error-reporting calls cold in BPI

Hal Finkel hfinkel at anl.gov
Fri Nov 8 12:33:06 PST 2013


----- Original Message -----
> ----- Original Message -----
> > 
> > On Nov 1, 2013, at 3:47 AM, Hal Finkel <hfinkel at anl.gov> wrote:
> > 
> > > ----- Original Message -----
> > >> 
> > >> 
> > >> 
> > >> On Thu, Oct 31, 2013 at 7:29 PM, Hal Finkel < hfinkel at anl.gov >
> > >> wrote:
> > >> 
> > >> 
> > >> 
> > >> ----- Original Message -----
> > >>> ----- Original Message -----
> > >>>> 
> > >>>> Actually looking at this patch, why not do this in the pass
> > >>>> which
> > >>>> adds function attributes by adding the cold attribute to error
> > >>>> reporting library functions?
> > >> 
> > >> Unless we're willing to mark all calls to fprintf as cold (which
> > >> I
> > >> don't think that we want to do), then I can't add the attributes
> > >> to
> > >> the function declarations. Only noreturn, nounwind, readonly and
> > >> readnone are currently supported as call-site function
> > >> attributes.
> > >> Being able to mark particular call sites as cold does seem to
> > >> make
> > >> sense, so maybe this is worth extending?
> > >> 
> > >> 
> > >> 
> > >> That had been my thought.
> > >> 
> > >> 
> > >> 
> > >> You're specifically referring to IPO/FunctionAttrs.cpp?
> > > 
> > > Okay, in that case I'm not sure that is the right place. For one
> > > thing, that pass runs very early (before we've run any constant
> > > propagation), which might make it less effective in this case.
> > > Also, if we just need to modify the call site, that is a local
> > > operation; it might be better just to put it in InstCombine, or
> > > something else that runs later. Since BPI is really only used by
> > > the SDAG Builder, maybe CGP would be a good place?
> > > 
> > > Thanks again,
> > > Hal
> > 
> > We're expecting BPI to eventually be used in various other
> > optimization passes, so CGP would be too late.
> 
> Okay, thanks! (Maybe this leaves me with InstCombine?) Does anything
> else use, or is expected to use, the cold attribute directly in
> addition to BPI? If not, maybe leaving this in BPI is just as good
> as anything else.

Chandler, please express an opinion. Would you want this in InstCombine? Thanks!

 -Hal

> 
>  -Hal
> 
> > 
> > > 
> > >> 
> > >> 
> > >> 
> > >> Yep.
> > > 
> > > --
> > > Hal Finkel
> > > Assistant Computational Scientist
> > > Leadership Computing Facility
> > > Argonne National Laboratory
> > > _______________________________________________
> > > llvm-commits mailing list
> > > llvm-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > 
> > 
> 
> --
> Hal Finkel
> Assistant Computational Scientist
> Leadership Computing Facility
> Argonne National Laboratory
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 

-- 
Hal Finkel
Assistant Computational Scientist
Leadership Computing Facility
Argonne National Laboratory



More information about the llvm-commits mailing list