Skip to content

Mutually recursive PIR function inlining#7688

Open
SeungheonOh wants to merge 2 commits intomasterfrom
sho/pirMutualRecInline
Open

Mutually recursive PIR function inlining#7688
SeungheonOh wants to merge 2 commits intomasterfrom
sho/pirMutualRecInline

Conversation

@SeungheonOh
Copy link
Copy Markdown
Collaborator

New optimization pass that will inline mutually recursive PIR functions when they satisfy below criteria.

Given chain of mutually recursive functions f, g, and h, we inline when

  • only one of the function is used in the body of recursive let
  • the other functions(helper) are used only single time the body of other function
  • All of the mutually recursive functions are not self recursive

@SeungheonOh SeungheonOh requested a review from zliu41 March 24, 2026 01:34
@github-actions
Copy link
Copy Markdown
Contributor

Execution Budget Golden Diff

c592a8d (master) vs 179a528

output

This comment will get updated when changes are made.

[ (constr 1
[ cse
, cse ]) ]))
, (constr 1
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we have issue similar to #7681 here. CSE not doing it's job probably due to some rename happening

match_Nat n {Bool} True (\(p : Nat) -> odd p))
(\(n : Nat) ->
match_Nat n {Bool} False (\(p : Nat) -> even p)))))
(\(s :
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was the existing test cases that was originally inefficient due to mutual recursion but now more efficient.

Copy link
Copy Markdown
Member

@zliu41 zliu41 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you check if this solves the problem described in Note [Making the cons case non-strict in caseList and caseList']?

asRecBinding = \case
TermBind bindAnn strictness decl rhs
| let arity = rhsArity rhs
, not (null arity) ->
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you require this condition? Can you just use the purity analysis instead?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think if it's a non-lambda term, it should be handled by inliner instead.

rhs' <- PLC.rename helperRhs
pure $ case saturates args (rhsArity rhs') of
Just Saturated -> fillAppContext rhs' args
_ -> term'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need to give up if it is OverSaturated?

This works across independent subgroups within the same @let rec@, e.g.
@{even, odd, f, g}@ where @{even, odd}@ and @{f, g}@ are separate cycles.

No beta reduction is performed here; the resulting unsaturated applications
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
No beta reduction is performed here; the resulting unsaturated applications
No beta reduction is performed here; the resulting applications

The resulting application isn't necessarily unsaturated, is it?

Nothing -> pure group

{-| Find a helper eligible for inlining: not a root, not self-recursive,
called from exactly one sibling, and used exactly once in that sibling. -}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These "exactly one" conditions seem quite restrictive. Can you use similar criteria as the main inliner - e.g., if it is used exactly once, then it's always OK. Otherwise, it is OK if costIsAcceptable and sizeIsAcceptable.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice. I'll do this

pure (host, helper)

{-| Inline all saturated calls to the helper within the host's RHS.
If successful (all references eliminated), remove the helper from the group. -}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline all saturated calls to the helper

Doesn't findCandidate guarantee that there's exactly one callsite?

And why must the callsite be saturated?

Just (hostKey, helperKey) ->
tryInline group hostKey helperKey >>= \case
Just group' -> collapseRecGroup group'
Nothing -> pure group
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that another candidate in the group can be inlined?

@SeungheonOh SeungheonOh self-assigned this Apr 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants