[clang] [llvm] [transforms] Inline simple variadic functions (PR #81058)

Jon Chesterfield via cfe-commits cfe-commits at lists.llvm.org
Thu Feb 15 07:12:50 PST 2024


JonChesterfield wrote:

> Not sure if this means isFunctionInlinable will go away in the followup patch, or if you plan to rewrite functions in a way that satisfies isFunctionInlinable. I think the end state should be that all functions go down the same codepath, not conditionally do something different based on whether they're "simple". I guess I don't have a strong preference for how you get there, though.

The logic I've got at present (which include the ABI rewriting) is

```C++
    bool usefulToSplit =
        splitFunctions() && (!F->isDeclaration() || rewriteABI());

    // F may already be a single basic block calling a known function
    // that takes a va_list, in which case it doens't need to be split.
    Function *Equivalent = isFunctionInlinable(M, F);

    if (usefulToSplit && !Equivalent) {
      Equivalent = DeriveInlinableVariadicFunctionPair(M, *F);
      assert(Equivalent);
      assert(isFunctionInlinable(M, *F)); // branch doesn't do this presently but it could do
      changed = true;
      functionToInliningTarget[F] = Equivalent;
    }
```

I'm not especially attached to the specific control flow.

The two transforms - inlining/call-rewrite and splitting an entry block off the variadic function function so that it can be inlined - are genuinely orthogonal and that is a really good property to preserve. It took some thought to get to that structure.

If we ignore that design and run functions through the block splitting unnecessarily, we win a combinatorial increase in required testing, a decrease in emitted code quality (spurious extra functions), an inability to pattern match on fprintf->vfprintf style code that happens to be in the application already. We would get to delete the isFunctionInlinable predicate.

The independent transform pipeline pattern is more important than the no special case branching heuristic. If it helps, view it as two complementary transforms where the one is skipped when it would be a no-op.

Related - if there's an objection to landing this as an inactive pass (only exercised by test code) we can put it into an optimisation pipeline immediately, it'll still remove some real world variadic calls even if the later patches don't make it.

https://github.com/llvm/llvm-project/pull/81058


More information about the cfe-commits mailing list