[PATCH] Consider error-reporting calls cold in BPI

Hal Finkel hfinkel at anl.gov
Thu Oct 31 19:29:10 PDT 2013


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

You're specifically referring to IPO/FunctionAttrs.cpp?

Thanks again,
Hal

> Then the existing BPI code will
> > trigger.
> > 
> > 
> > Also, I would sink the logic for determining if a particular
> > library
> > call to write to a stream is writing to 'stderr' into
> > TargetLibraryInfo itself. Essentially, make a boolean query for
> > 'isLikelyErrorReportingFn' or something.
> 
> Make sense.
> 
> > 
> > 
> > Finally, why not also bias against write() calls with a numeric
> > argument pointing to the typical stderr file descriptor?
> 
> I could do that too.
> 
> > 
> > 
> > With all of these heuristics, it would be useful to clarify that
> > this
> > isn't a *guarantee* that this is an error reporting function (you
> > can rearrange stderr if you want, etc), but it's a strong signal
> > that we can do stuff like synthesizing the cold attribute from.
> 
> Yes, exactly.
> 
> Thanks again,
> Hal
> 
> > 
> > 
> > 
> > On Sun, Oct 27, 2013 at 4:23 PM, Hal Finkel < hfinkel at anl.gov >
> > wrote:
> > 
> > 
> > Hello,
> > 
> > BranchProbabilityInfo, which is used by the SDAG builder to assign
> > branch weights, contains a number of well-known heuristics for
> > statically predicting branch outcomes. One of these heuristics
> > assigns low probabilities to paths post-dominated by cold calls
> > (calls with the cold attribute). This patch enhances that heuristic
> > to consider 'error reporting' calls to also be cold. An 'error
> > reporting' call is a call to perror or a call to fprintf, fwrite,
> > etc. where stderr is the stream.
> > 
> > This is one of the suggested heuristics from:
> > http://impact.crhc.illinois.edu/shared/papers/pact-98-branchpred.pdf
> > 
> > I don't see any significant test-suite changes.
> > 
> > Please review.
> > 
> > Thanks again,
> > Hal
> > 
> > --
> > 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