[PATCH] Consider error-reporting calls cold in BPI

[Hal Finkel]

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

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