[PATCH] Consider error-reporting calls cold in BPI

[Hal Finkel]

----- Original Message -----
> ----- Original Message -----
> > 
> > 
> > 
> > On Mon, Nov 11, 2013 at 10:04 PM, Hal Finkel < hfinkel at anl.gov >
> > wrote:
> > 
> > 
> > 
> > 
> > ----- Original Message -----
> > > 
> > > 
> > > 
> > > 
> > > On Sat, Nov 9, 2013 at 5:44 AM, Hal Finkel < hfinkel at anl.gov >
> > > wrote:
> > > 
> > > 
> > > Okay, I've attached a patch that adds this into SimplifyLibCalls.
> > > In
> > > this incarnation, I've added the functionality at the top-level
> > > of
> > > SimplifyLibCalls itself (which makes it different from all of the
> > > other simplifications). My rationale for this is that adding the
> > > cold hint should not depend on the builtin status of the
> > > functions.
> > > I'm not sure how much I like this. Thoughts?
> > > I think it should depend on the builtin status of the functions.
> > > If
> > > we're in a freestanding environment, there is no reason to
> > > believe
> > > that the meanings for the various file descriptors continue to
> > > hold.
> > 
> > Chandler, Meador, et al.,
> > 
> > Thanks for your feedback! I've attached a new version (which makes
> > this look much like all of the other optimizations). Please review.
> > 
> > 
> > 
> > While I deeply question the fundamental design here, I suspect your
> > patch should follow it.
> > 
> 
> Yea, also not my favorite ;)
> 
> > 
> > + ErrorReportingOpt ER(/* StreamArg = */ 0);
> > + (void) ER.callOptimizer(Callee, CI, B);
> > +
> > 
> > 
> > The way the rest of the simplify lib calls works is for specific
> > lib
> > calls to subclass a more more specialized base class to provide
> > delegated optimizations, not to build an instance of the class...
> 
> Heh... I was specifically trying to follow the existing pattern here.
> For example:
> 
> struct CosOpt : public UnsafeFPLibCallOptimization {
>   CosOpt(bool UnsafeFPShrink) :
>   UnsafeFPLibCallOptimization(UnsafeFPShrink) {}
>   virtual Value *callOptimizer(Function *Callee, CallInst *CI,
>   IRBuilder<> &B) {
>     Value *Ret = NULL;
>     if (UnsafeFPShrink && Callee->getName() == "cos" &&
>         TLI->has(LibFunc::cosf)) {
>       UnaryDoubleFPOpt UnsafeUnaryDoubleFP(true);
>       Ret = UnsafeUnaryDoubleFP.callOptimizer(Callee, CI, B);
>     }
> ...
> 
> > 
> > 
> > Bah, maybe just put this in a couple of static function and use a
> > more boring design? Seems simpler.
> 
> I could certainly move all of the real logic in ErrorReportingOpt
> into a static member function, and call it from the other three
> places. Or would you prefer that I move the whole thing into a
> static function that is called from
> LibCallSimplifierImpl::lookupOptimization (or elsewhere)?

Chandler, are you okay with me committing this as is? As noted, I think that it matches the existing design.

Thanks again,
Hal

> 
> Thanks again,
> Hal
> 
> > 
> > 
> > -Chandler
> > 
> > 
> > 
> > 
> > -Hal
> > 
> > 
> > 
> > --
> > Hal Finkel
> > Assistant Computational Scientist
> > Leadership Computing Facility
> > Argonne National Laboratory
> > 
> > 
> 
> --
> 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