[llvm] r293559 - Expose isLegalToPromot as a global helper function so that SamplePGO pass can call it for legality check.

Benjamin Kramer via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 30 15:51:03 PST 2017


On Mon, Jan 30, 2017 at 11:46 PM, Dehao Chen via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: dehao
> Date: Mon Jan 30 16:46:37 2017
> New Revision: 293559
>
> URL: http://llvm.org/viewvc/llvm-project?rev=293559&view=rev
> Log:
> Expose isLegalToPromot as a global helper function so that SamplePGO pass can call it for legality check.
>
> Summary: SamplePGO needs to check if it is legal to promote a target before it actually promotes it.
>
> Reviewers: davidxl
>
> Reviewed By: davidxl
>
> Subscribers: llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D29306
>
> Modified:
>     llvm/trunk/include/llvm/Transforms/Instrumentation.h
>     llvm/trunk/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp
>
> Modified: llvm/trunk/include/llvm/Transforms/Instrumentation.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Transforms/Instrumentation.h?rev=293559&r1=293558&r2=293559&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/Transforms/Instrumentation.h (original)
> +++ llvm/trunk/include/llvm/Transforms/Instrumentation.h Mon Jan 30 16:46:37 2017
> @@ -88,6 +88,10 @@ ModulePass *
>  createPGOInstrumentationUseLegacyPass(StringRef Filename = StringRef(""));
>  ModulePass *createPGOIndirectCallPromotionLegacyPass(bool InLTO = false);
>
> +// Helper function to check if it is legal to promote indirect call \p Inst
> +// to a direct call of function \p F. Stores the reason in \p Reason.
> +bool isLegalToPromote(Instruction *Inst, Function *F, const char **Reason);

Please chose a different name if you want this in the llvm namespace,
this is likely going to clash as there are many promotion things in
LLVM.

The Reason thing here is really ugly, is it really needed to expose
that? What would a user do with a purely textual reason?

> +
>  // Helper function that transforms Inst (either an indirect-call instruction, or
>  // an invoke instruction , to a conditional call to F. This is like:
>  //     if (Inst.CalledValue == F)
>
> Modified: llvm/trunk/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp?rev=293559&r1=293558&r2=293559&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp (original)
> +++ llvm/trunk/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp Mon Jan 30 16:46:37 2017
> @@ -144,17 +144,9 @@ private:
>    // defines.
>    InstrProfSymtab *Symtab;
>
> -  enum TargetStatus {
> -    OK,                   // Should be able to promote.
> -    NotAvailableInModule, // Cannot find the target in current module.
> -    ReturnTypeMismatch,   // Return type mismatch b/w target and indirect-call.
> -    NumArgsMismatch,      // Number of arguments does not match.
> -    ArgTypeMismatch       // Type mismatch in the arguments (cannot bitcast).
> -  };
> -
>    // Test if we can legally promote this direct-call of Target.
> -  TargetStatus isPromotionLegal(Instruction *Inst, uint64_t Target,
> -                                Function *&F);
> +  bool isPromotionLegal(Instruction *Inst, uint64_t Target, Function *&F,
> +                        const char **Reason = nullptr);
>
>    // A struct that records the direct target and it's call count.
>    struct PromotionCandidate {
> @@ -178,22 +170,6 @@ private:
>                          const std::vector<PromotionCandidate> &Candidates,
>                          uint64_t &TotalCount);
>
> -  static const char *StatusToString(const TargetStatus S) {
> -    switch (S) {
> -    case OK:
> -      return "OK to promote";
> -    case NotAvailableInModule:
> -      return "Cannot find the target";
> -    case ReturnTypeMismatch:
> -      return "Return type mismatch";
> -    case NumArgsMismatch:
> -      return "The number of arguments mismatch";
> -    case ArgTypeMismatch:
> -      return "Argument Type mismatch";
> -    }
> -    llvm_unreachable("Should not reach here");
> -  }
> -
>    // Noncopyable
>    ICallPromotionFunc(const ICallPromotionFunc &other) = delete;
>    ICallPromotionFunc &operator=(const ICallPromotionFunc &other) = delete;
> @@ -207,43 +183,58 @@ public:
>  };
>  } // end anonymous namespace
>
> -ICallPromotionFunc::TargetStatus
> -ICallPromotionFunc::isPromotionLegal(Instruction *Inst, uint64_t Target,
> -                                     Function *&TargetFunction) {
> -  Function *DirectCallee = Symtab->getFunction(Target);
> -  if (DirectCallee == nullptr)
> -    return NotAvailableInModule;
> +bool llvm::isLegalToPromote(Instruction *Inst, Function *F,
> +                            const char **Reason) {
>    // Check the return type.
>    Type *CallRetType = Inst->getType();
>    if (!CallRetType->isVoidTy()) {
> -    Type *FuncRetType = DirectCallee->getReturnType();
> +    Type *FuncRetType = F->getReturnType();
>      if (FuncRetType != CallRetType &&
> -        !CastInst::isBitCastable(FuncRetType, CallRetType))
> -      return ReturnTypeMismatch;
> +        !CastInst::isBitCastable(FuncRetType, CallRetType)) {
> +      if (Reason)
> +        *Reason = "Return type mismatch";
> +      return false;
> +    }
>    }
>
>    // Check if the arguments are compatible with the parameters
> -  FunctionType *DirectCalleeType = DirectCallee->getFunctionType();
> +  FunctionType *DirectCalleeType = F->getFunctionType();
>    unsigned ParamNum = DirectCalleeType->getFunctionNumParams();
>    CallSite CS(Inst);
>    unsigned ArgNum = CS.arg_size();
>
> -  if (ParamNum != ArgNum && !DirectCalleeType->isVarArg())
> -    return NumArgsMismatch;
> +  if (ParamNum != ArgNum && !DirectCalleeType->isVarArg()) {
> +    if (Reason)
> +      *Reason = "The number of arguments mismatch";
> +    return false;
> +  }
>
>    for (unsigned I = 0; I < ParamNum; ++I) {
>      Type *PTy = DirectCalleeType->getFunctionParamType(I);
>      Type *ATy = CS.getArgument(I)->getType();
>      if (PTy == ATy)
>        continue;
> -    if (!CastInst::castIsValid(Instruction::BitCast, CS.getArgument(I), PTy))
> -      return ArgTypeMismatch;
> +    if (!CastInst::castIsValid(Instruction::BitCast, CS.getArgument(I), PTy)) {
> +      if (Reason)
> +        return "Argument Type mismatch";
> +      return false;
> +    }
>    }
>
>    DEBUG(dbgs() << " #" << NumOfPGOICallPromotion << " Promote the icall to "
> -               << Symtab->getFuncName(Target) << "\n");
> -  TargetFunction = DirectCallee;
> -  return OK;
> +               << F->getName() << "\n");
> +  return true;
> +}
> +
> +bool ICallPromotionFunc::isPromotionLegal(Instruction *Inst, uint64_t Target,
> +                                          Function *&TargetFunction,
> +                                          const char **Reason) {
> +  TargetFunction = Symtab->getFunction(Target);
> +  if (TargetFunction == nullptr) {
> +    *Reason = "Cannot find the target";
> +    return false;
> +  }
> +  return isLegalToPromote(Inst, TargetFunction, Reason);
>  }
>
>  // Indirect-call promotion heuristic. The direct targets are sorted based on
> @@ -283,10 +274,9 @@ ICallPromotionFunc::getPromotionCandidat
>        break;
>      }
>      Function *TargetFunction = nullptr;
> -    TargetStatus Status = isPromotionLegal(Inst, Target, TargetFunction);
> -    if (Status != OK) {
> +    const char *Reason = nullptr;
> +    if (!isPromotionLegal(Inst, Target, TargetFunction, &Reason)) {
>        StringRef TargetFuncName = Symtab->getFuncName(Target);
> -      const char *Reason = StatusToString(Status);
>        DEBUG(dbgs() << " Not promote: " << Reason << "\n");
>        emitOptimizationRemarkMissed(
>            F.getContext(), "pgo-icall-prom", F, Inst->getDebugLoc(),
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list