[PATCH] Consider error-reporting calls cold in BPI

Hal Finkel hfinkel at anl.gov
Mon Nov 4 10:43:00 PST 2013


----- 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.

 -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



More information about the llvm-commits mailing list