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.

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

Tail call miscompiled by FastISel #53243

Closed
nikic opened this issue Jan 17, 2022 · 3 comments
Closed

Tail call miscompiled by FastISel #53243

nikic opened this issue Jan 17, 2022 · 3 comments
Labels
llvm:codegen

Comments

@nikic
Copy link
Contributor

nikic commented Jan 17, 2022

Running https://gist.github.com/nikic/b6c45e5eeac03ef1fd842353815ceba4 through llc -O0 results in an entirely empty function, while it should have a jmp set_state@PLT.

It works fine without the dbg.value, or without the tail annotation. It also works fine with -fast-isel=0.
@nikic nikic mentioned this issue Jan 17, 2022
Closed
@nikic
Copy link
Contributor Author

nikic commented Jan 17, 2022

It looks like the TCRETURN gets emitted properly, but then gets dropped by this piece of code:

llvm-project/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp

Lines 1564 to 1570 in 197f3c0

// If the call was emitted as a tail call, we're done with the block.
// We also need to delete any previously emitted instructions.
if (HadTailCall) {
FastIS->removeDeadCode(SavedInsertPt, FuncInfo->MBB->end());
--BI;
break;
}

@nikic
Copy link
Contributor Author

nikic commented Jan 17, 2022

This is due to e297006 (cc @fhahn). The DBG_VALUE produced by FastISel gets moved before the TCRETURN, which means that the code that was intended to delete the instructions produced by FastISel now also kills the TCRETURN.

@nikic
Copy link
Contributor Author

nikic commented Jan 17, 2022

Candidate patch: https://reviews.llvm.org/D117489

@nikic nikic closed this as completed in e7c9a6c Jan 24, 2022
mem-frob pushed a commit to draperlaboratory/hope-llvm-project that referenced this issue Oct 7, 2022
@nikic
[SDAG] Don't move DBG_VALUE instructions after insertion point during…
6109bd0 
… scheduling (PR53243)

EmitSchedule() shouldn't be touching instructions after the provided
insertion point. The change introduced in D83561 performs a scan to
the end of the block, and thus may move unrelated instructions. In
particular, this ends up moving instructions that have been produced
by FastISel and will later be deleted. Moving them means that more
instructions than intended are removed.

Fix this by stopping the iteration when the insertion point is
reached.

Fixes llvm/llvm-project#53243.

Differential Revision: https://reviews.llvm.org/D117489
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
llvm:codegen
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@nikic