Re: [PATCH v2 15/50] helper-to-tcg: PrepareForOptPass, cull unused functions

[Alessandro Di Federico via qemu development <qemu-devel@nongnu.org>]

On Thu, 30 Jul 2026 05:09:49 +0200
Anton Johansson <anjo@rev.ng> wrote:

> Make an early pass over all functions in the input module and filter out
> functions with:
> 
>   1. Invalid return type, or;
>   2. No helper-to-tcg annotation and not called by a function with such
>      a annotation.
> 
> A commandline option is also added to force translation of all functions
> starting with "helper_".
> 
> Signed-off-by: Anton Johansson <anjo@rev.ng>
> ---
>  .../helper-to-tcg/include/CmdLineOptions.hpp  |  2 +
>  .../include/PrepareForOptPass.hpp             |  7 +-
>  subprojects/helper-to-tcg/src/Pipeline.cpp    |  5 ++
>  .../PrepareForOptPass/PrepareForOptPass.cpp   | 86 +++++++++++++++++++
>  4 files changed, 96 insertions(+), 4 deletions(-)
> 
> diff --git a/subprojects/helper-to-tcg/include/CmdLineOptions.hpp b/subprojects/helper-to-tcg/include/CmdLineOptions.hpp
> index 93706b78c5..ca1cb59835 100644
> --- a/subprojects/helper-to-tcg/include/CmdLineOptions.hpp
> +++ b/subprojects/helper-to-tcg/include/CmdLineOptions.hpp
> @@ -21,3 +21,5 @@
>  
>  // Options for pipeline
>  extern llvm::cl::list<std::string> InputFiles;
> +// Options for PrepareForOptPass
> +extern llvm::cl::opt<bool> TranslateAllHelpers;
> diff --git a/subprojects/helper-to-tcg/include/PrepareForOptPass.hpp b/subprojects/helper-to-tcg/include/PrepareForOptPass.hpp
> index e007243578..08ca9a43bb 100644
> --- a/subprojects/helper-to-tcg/include/PrepareForOptPass.hpp
> +++ b/subprojects/helper-to-tcg/include/PrepareForOptPass.hpp
> @@ -29,11 +29,10 @@
>  
>  class PrepareForOptPass : public llvm::PassInfoMixin<PrepareForOptPass> {
>      AnnotationMapTy &ResultAnnotations;
> -public:
> +
> +  public:
>      PrepareForOptPass(AnnotationMapTy &ResultAnnotations)
> -        : ResultAnnotations(ResultAnnotations)
> -    {
> -    }
> +        : ResultAnnotations(ResultAnnotations) {}
>      llvm::PreservedAnalyses run(llvm::Module &M,
>                                  llvm::ModuleAnalysisManager &MAM);
>  };
> diff --git a/subprojects/helper-to-tcg/src/Pipeline.cpp b/subprojects/helper-to-tcg/src/Pipeline.cpp
> index 051611b0f3..89637eaec6 100644
> --- a/subprojects/helper-to-tcg/src/Pipeline.cpp
> +++ b/subprojects/helper-to-tcg/src/Pipeline.cpp
> @@ -65,6 +65,11 @@ static cl::opt<std::string>
>                cl::init(""), cl::cat(Cat));
>  #endif
>  
> +// Options for PrepareForOptPass
> +cl::opt<bool> TranslateAllHelpers(
> +    "translate-all-helpers", cl::init(false),
> +    cl::desc("Translate all functions starting with helper_*"), cl::cat(Cat));
> +
>  // Define a TargetTransformInfo (TTI) subclass, this allows for overriding
>  // common per-llvm-target information expected by other LLVM passes, such
>  // as the width of the largest scalar/vector registers.  Needed for consistent
> diff --git a/subprojects/helper-to-tcg/src/PrepareForOptPass/PrepareForOptPass.cpp b/subprojects/helper-to-tcg/src/PrepareForOptPass/PrepareForOptPass.cpp
> index 1228ac952f..df6d9eeec8 100644
> --- a/subprojects/helper-to-tcg/src/PrepareForOptPass/PrepareForOptPass.cpp
> +++ b/subprojects/helper-to-tcg/src/PrepareForOptPass/PrepareForOptPass.cpp
> @@ -16,17 +16,25 @@
>  //
>  
>  #include "PrepareForOptPass.hpp"
> +#include "CmdLineOptions.hpp"
>  #include "Error.hpp"
> +#include "FunctionAnnotation.hpp"
> +#include "LlvmCompat.hpp"
>  
> +#include <llvm/ADT/SmallPtrSet.h>
>  #include <llvm/ADT/StringRef.h>
>  #include <llvm/ADT/StringSet.h>
>  #include <llvm/Demangle/Demangle.h>
>  #include <llvm/IR/Constants.h>
>  #include <llvm/IR/Function.h>
>  #include <llvm/IR/Instruction.h>
> +#include <llvm/IR/Instructions.h>
>  #include <llvm/IR/Module.h>
>  #include <llvm/Support/Debug.h>
>  
> +#include <queue>
> +#include <set>
> +
>  #define DEBUG_TYPE "prepare-for-opt"
>  
>  using namespace llvm;
> @@ -156,9 +164,87 @@ static void collectAnnotations(Module &M, AnnotationMapTy &ResultAnnotations) {
>      });
>  }
>  
> +inline bool hasValidReturnTy(const Module &M, const Function *F) {
> +    Type *RetTy = F->getReturnType();
> +    return RetTy->isStructTy() || RetTy == Type::getVoidTy(F->getContext()) ||
> +           RetTy == Type::getInt8Ty(M.getContext()) ||
> +           RetTy == Type::getInt16Ty(M.getContext()) ||
> +           RetTy == Type::getInt32Ty(M.getContext()) ||
> +           RetTy == Type::getInt64Ty(M.getContext());
> +}
> +
> +// Functions that should be removed:
> +//   - No helper-to-tcg annotation (if TranslateAllHelpers == false);
> +//   - Invalid (non-integer/void) return type
> +static bool shouldRemoveFunction(const Module &M, const Function &F,
> +                                 const AnnotationMapTy &AnnotationMap) {
> +    if (F.isDeclaration()) {
> +        return false;
> +    }
> +
> +    if (!hasValidReturnTy(M, &F)) {

I'd do this check after the others and write something to `llvm::errs()`.
If a helper has been explicitly marked as to be translated, but it's
unsuitable, we need to notify the user.

> +        return true;
> +    }
> +
> +    std::queue<const Function *> Worklist;
> +    std::set<const Function *> Visited;
> +    Worklist.push(&F);
> +    while (!Worklist.empty()) {
> +        const Function *F = Worklist.front();
> +        Worklist.pop();
> +        if (F->isDeclaration() or Visited.find(F) != Visited.end()) {
> +            continue;
> +        }
> +        Visited.insert(F);
> +
> +        if (TranslateAllHelpers and
> +            compat::isFunctionQemuHelper(F->getName())) {
> +            // If --translate-all-helpers is provided and `F` starts with
> +            // "helper_*", then don't skip it.
> +            return false;
> +        } else if (auto It = AnnotationMap.find(F); It != AnnotationMap.end()) {
> +            // Otherwise check "helper-to-tcg" annotation.
> +            const Annotations &Ann = It->second;
> +            if (Ann.isSet(FunctionAnnotation::HelperToTcg)) {
> +                return false;
> +            }
> +        }
> +
> +        // Push functions that call `F` to the worklist, this way we retain
> +        // functions that are being called by functions with the "helper-to-tcg"
> +        // annotation.
> +        for (const User *U : F->users()) {
> +            auto Call = dyn_cast<CallInst>(U);

Use `CallBase` so you also catch `InvokeInst`.

> +            if (!Call) {
> +                continue;
> +            }
> +            const Function *ParentF = Call->getParent()->getParent();
> +            Worklist.push(ParentF);
> +        }
> +    }
> +
> +    return true;
> +}
> +
> +static void cullUnusedFunctions(Module &M, AnnotationMapTy &Annotations) {

I'd call it `purgeUnusedFunctions`, but this is fine as well.

> +    SmallPtrSet<Function *, 16> FunctionsToRemove;
> +    for (auto &F : M) {
> +        if (shouldRemoveFunction(M, F, Annotations)) {
> +            FunctionsToRemove.insert(&F);
> +        }
> +    }

This is not super efficient, for every function you walk the call graph.

I'd walk the list of functions and mark those we need to keep due to
name/annotation.
Then I'd walk the CallGraph (check out CallGraph.h) starting from them
(and stopping if you get to a function you already visited.

It'd also be nice to fail in case we have indirect calls, at some point.

> +
> +    for (Function *F : FunctionsToRemove) {
> +        Annotations.erase(F);
> +        F->setComdat(nullptr);
> +        F->deleteBody();
> +    }
> +}
> +
>  PreservedAnalyses PrepareForOptPass::run(Module &M,
>                                           ModuleAnalysisManager &MAM) {
>      demangleFunctionNames(M);
>      collectAnnotations(M, ResultAnnotations);
> +    cullUnusedFunctions(M, ResultAnnotations);
>      return PreservedAnalyses::none();
>  }
> -- 
> 2.52.0

Reviewed-by: Alessandro Di Federico <ale@rev.ng>

-- 
Alessandro Di Federico
rev.ng Labs