Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BOLT][RISCV] Handle long tail calls #67098

Merged
merged 1 commit into from
Oct 5, 2023
Merged

Conversation

mtvec
Copy link
Contributor

@mtvec mtvec commented Sep 22, 2023

Long tail calls use the following instruction sequence on RISC-V:

1: auipc xi, %pcrel_hi(sym)
jalr zero, %pcrel_lo(1b)(xi)

Since the second instruction in isolation looks like an indirect branch, this confused BOLT and most functions containing a long tail call got marked with "unknown control flow" and didn't get optimized as a consequence.

This patch fixes this by detecting long tail call sequence in analyzeIndirectBranch. FixRISCVCallsPass also had to be updated to expand long tail calls to PseudoTAIL instead of PseudoCALL.

Besides this, this patch also fixes a minor issue with compressed tail calls (c.jr) not being detected.

Note that I had to change BinaryFunction::postProcessIndirectBranches slightly: the documentation of MCPlusBuilder::analyzeIndirectBranch mentions that the [Begin, End) range contains the instructions immediately preceding Instruction. However, in
postProcessIndirectBranches, all the instructions in the BB where passed in the range. This made it difficult to find the preceding instruction so I made sure only the preceding instructions are passed.

Long tail calls use the following instruction sequence on RISC-V:

```
1: auipc xi, %pcrel_hi(sym)
jalr zero, %pcrel_lo(1b)(xi)
```

Since the second instruction in isolation looks like an indirect branch,
this confused BOLT and most functions containing a long tail call got
marked with "unknown control flow" and didn't get optimized as a
consequence.

This patch fixes this by detecting long tail call sequence in
`analyzeIndirectBranch`. `FixRISCVCallsPass` also had to be updated to
expand long tail calls to `PseudoTAIL` instead of `PseudoCALL`.

Besides this, this patch also fixes a minor issue with compressed tail
calls (`c.jr`) not being detected.

Note that I had to change `BinaryFunction::postProcessIndirectBranches`
slightly: the documentation of `MCPlusBuilder::analyzeIndirectBranch`
mentions that the [`Begin`, `End`) range contains the instructions
immediately preceding `Instruction`. However, in
`postProcessIndirectBranches`, *all* the instructions in the BB where
passed in the range. This made it difficult to find the preceding
instruction so I made sure *only* the preceding instructions are passed.
@mtvec
Copy link
Contributor Author

mtvec commented Oct 4, 2023

Ping

Copy link
Contributor

@rafaelauler rafaelauler left a comment

Choose a reason for hiding this comment

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

LGTM

@mtvec mtvec merged commit 7fa3377 into llvm:main Oct 5, 2023
@mtvec mtvec deleted the bolt-riscv-long-tail branch October 10, 2023 09:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants