This is an archive of the discontinued LLVM Phabricator instance.

[X86][Win64] Avoid statepoints in trailing call position
ClosedPublic

Authored by zero9178 on Feb 12 2022, 11:41 AM.

Details

Summary

The "avoid trailing call pass" makes sure that no function ends with a call instruction for the purpose of the unwinder.
It starts of by skipping over any non real instruction, which is approximated via the Pseudo and Meta property. This sadly leads to issues when the last machine instruction is a STATEPOINT, as it is skipped despite it lowering to a call.

This patch fixes the use of a statepoint in the trailing call position by making sure it is not skipped.

Diff Detail

Event Timeline

zero9178 created this revision.Feb 12 2022, 11:41 AM
zero9178 requested review of this revision.Feb 12 2022, 11:41 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 12 2022, 11:41 AM
sanjoy removed a reviewer: sanjoy.Feb 12 2022, 12:00 PM

LGTM in the sense that this doesn't look obviously wrong, but that I don't understand the Win64 code here. You should probably confirm with someone more knowledgeable of that area.

llvm/test/CodeGen/X86/win64-eh-trailing-statepoint.ll
3

Can you use update_lit_test here? It's really hard to see what the resulting assembly actually is here.

zero9178 updated this revision to Diff 408605.Feb 14 2022, 1:53 PM

Address reviewers comments:

  • Make use of update_llc_test_checks.py script to generate the output checks
zero9178 marked an inline comment as done.Feb 14 2022, 1:53 PM
rnk accepted this revision.Feb 14 2022, 1:55 PM

lgtm

I have some other changes I want to make, but just go ahead and land this and I'll do them as a follow-up.

llvm/lib/Target/X86/X86AvoidTrailingCall.cpp
70–71

This comment about being "conservative" isn't correct anymore. I'll follow-up and fix it.

73

So, statepoints are pseudo instructions that are later lowered to calls. Does the MI.isCall() predicate return true for statepoints? Could we use this condition?

return !MI.isCall() && !MI.isPseudo() && !MI.isMetaInstruction();
This revision is now accepted and ready to land.Feb 14 2022, 1:55 PM
zero9178 marked an inline comment as done.Feb 14 2022, 2:13 PM
zero9178 added inline comments.
llvm/lib/Target/X86/X86AvoidTrailingCall.cpp
73

It does so using that property would probably be a good idea and be more generic! I am assuming you mean
MI.isCall() || (!MI.isPseudo() && !MI.isMetaInstruction()) though, so that the code below would find the first real or call instruction

zero9178 updated this revision to Diff 408617.Feb 14 2022, 2:14 PM
zero9178 marked an inline comment as done.

Makes use of the isCall property of a machine instruction instead of specializing for statepoint in particular.
That way the code fixes the issue for any pseudo instruction that lowers to a call.

This revision was landed with ongoing or failed builds.Feb 15 2022, 3:17 AM
This revision was automatically updated to reflect the committed changes.