Skip to content

Additional tce tests #144650

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Additional tce tests #144650

wants to merge 1 commit into from
+90 −2

Conversation

Borgerr
Copy link
Contributor

@Borgerr Borgerr commented Jul 29, 2025 •
edited by rustbot
Loading

r? @oli-obk

Adds known-bug tests for LLVM emissions regarding indirect operands for TCE. Also includes a test, indexer.rs, referring to function_table behavior described by the RFC.

Depends on #144232

Closes #144293

@rustbot
Copy link
Collaborator

rustbot commented Jul 29, 2025

oli-obk is not on the review rotation at the moment.
They may take a while to respond.

@rustbot rustbot added A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jul 29, 2025
@rustbot
Copy link
Collaborator

rustbot commented Jul 29, 2025

Some changes occurred in compiler/rustc_codegen_ssa

cc @WaffleLapkin

Some changes occurred in compiler/rustc_codegen_gcc

cc @antoyo, @GuillaumeGomez

@rustbot

This comment has been minimized.

Sign in to view
@Borgerr Borgerr force-pushed the additional-tce-tests branch from cdfbab1 to 06db3a5 Compare July 29, 2025 20:02
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Borgerr Borgerr force-pushed the additional-tce-tests branch from 06db3a5 to 0dba4bb Compare July 29, 2025 20:52
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@oli-obk
Copy link
Contributor

oli-obk commented Jul 30, 2025

You're gonna need the kind of //@normalize-stderr attributes as other ICE tests have, or just add the test to tests/crashes

@Borgerr Borgerr force-pushed the additional-tce-tests branch from 0dba4bb to e0738ad Compare July 30, 2025 14:49
@rust-log-analyzer

This comment has been minimized.

Sign in to view
@Borgerr Borgerr force-pushed the additional-tce-tests branch from e0738ad to e0b4595 Compare July 30, 2025 16:55
WaffleLapkin
WaffleLapkin reviewed Jul 30, 2025
View reviewed changes
tests/ui/explicit-tail-calls/indexer.rs Outdated
Comment on lines 1 to 3
// Indexing taken from
// https://github.com/phi-go/rfcs/blob/guaranteed-tco/text%2F0000-explicit-tail-calls.md#tail-call-elimination
// should probably come back to after some decision on verbiage
Copy link
Member

Choose a reason for hiding this comment

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

What do you mean by "decision on verbiage"?.. What is this test even testing?

you can make the test pass (hopefully) by just replacing &dyn Fn(usize) with fn(usize)

Copy link
Contributor Author

@Borgerr Borgerr Aug 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

"Decision on verbiage" was meant to indicate more discussion on examples given in that part of the RFC. From what I could see, nobody had tested that particular example of tail-calling something from a function table, and I was curious as to whether it'd work as desired, as it's kind of an operation. Didn't look too close in that regard though, admittedly.
I can indicate that better with a new comment, thanks for pointing that out.

Test also does pass with the described change. Push coming in a moment. However, it does feel a little silly to include now, so if input indicates I should remove it, I will. I suppose another approach that would include indexing would be something like a "basic examples from RFC" sort of file, or just appending this behavior to another test.

@Borgerr Borgerr force-pushed the additional-tce-tests branch from e0b4595 to 1f4a88a Compare August 2, 2025 16:31
WaffleLapkin
WaffleLapkin reviewed Aug 2, 2025
View reviewed changes
@Borgerr Borgerr force-pushed the additional-tce-tests branch from 1f4a88a to c9898dc Compare August 3, 2025 00:17
@WaffleLapkin
Copy link
Member

@Borgerr I think you broke something in rebase, since you have a commit which is not yours...

@WaffleLapkin WaffleLapkin added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 3, 2025
@Borgerr Borgerr force-pushed the additional-tce-tests branch from c9898dc to caa3cf1 Compare August 3, 2025 14:50
@Borgerr
Copy link
Contributor Author

Borgerr commented Aug 3, 2025

@rustbot label -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 3, 2025
@WaffleLapkin
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Aug 3, 2025

📌 Commit caa3cf1 has been approved by WaffleLapkin

It is now in the queue for this repository.

@bors bors removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 3, 2025
@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 3, 2025
@WaffleLapkin
Copy link
Member

@bors rollup

samueltardieu added a commit to samueltardieu/rust that referenced this pull request Aug 3, 2025
@samueltardieu
Rollup merge of rust-lang#144650 - Borgerr:additional-tce-tests, r=Wa…
1f2a633 
…ffleLapkin

Additional tce tests

r? `@oli-obk`

Adds known-bug tests for LLVM emissions regarding indirect operands for TCE. Also includes a test, `indexer.rs`, referring to function_table behavior described by the RFC.

Depends on rust-lang#144232

Closes rust-lang#144293
samueltardieu added a commit to samueltardieu/rust that referenced this pull request Aug 3, 2025
@samueltardieu
Rollup merge of rust-lang#144650 - Borgerr:additional-tce-tests, r=Wa…
4010404 
…ffleLapkin

Additional tce tests

r? ``@oli-obk``

Adds known-bug tests for LLVM emissions regarding indirect operands for TCE. Also includes a test, `indexer.rs`, referring to function_table behavior described by the RFC.

Depends on rust-lang#144232

Closes rust-lang#144293
@samueltardieu samueltardieu mentioned this pull request Aug 3, 2025
Closed
bors added a commit that referenced this pull request Aug 3, 2025
@bors
Auto merge of #144860 - samueltardieu:rollup-3i49mfl, r=samueltardieu
f9cd680 
Rollup of 11 pull requests

Successful merges:

 - #142678 (Misc cleanups of `generic_arg_infer` related HIR logic)
 - #144650 (Additional tce tests)
 - #144738 (Remove the omit_gdb_pretty_printer_section attribute)
 - #144790 (Multiple bounds checking elision failures)
 - #144805 (compiletest: Preliminary cleanup of `ProcRes` printing/unwinding)
 - #144808 (`Interner` arg to `EarlyBinder` does not affect auto traits)
 - #144816 (Update E0562 to account for the new impl trait positions)
 - #144822 (Return a struct with named fields from `hash_owner_nodes`)
 - #144824 (Updated test links in compiler)
 - #144829 (Use full flag name in strip command for Darwin)
 - #144843 (Weekly `cargo update`)

Failed merges:

 - #144794 (Port `#[coroutine]` to the new attribute system)

r? `@ghost`
`@rustbot` modify labels: rollup
@samueltardieu
Copy link
Member

#144860 (comment)
@bors r-

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 3, 2025
@Borgerr
explicit tail call tests with indirect operands in LLVM, small test f…
1fe1bf5 
…or indexing into a function table as described by RFC 3407
@Borgerr Borgerr force-pushed the additional-tce-tests branch from caa3cf1 to 1fe1bf5 Compare August 3, 2025 20:12
@Borgerr
Copy link
Contributor Author

Borgerr commented Aug 3, 2025

@rustbot label -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Aug 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@WaffleLapkin WaffleLapkin WaffleLapkin approved these changes

Assignees

@oli-obk oli-obk

Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Tail calls with indirect operands are untested
7 participants
@Borgerr @rustbot @rust-log-analyzer @oli-obk @WaffleLapkin @bors @samueltardieu