Skip to content

[cherry-pick][SelectionDAG] Disable FastISel for swiftasync functions (#70741) #7787

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

Conversation

felipepiovezan
Copy link

Most (x86) swiftasync functions tend to use both SelectionDAGISel and FastISel lowering:

  • FastISel argument lowering can only handle C calling convention.
  • FastISel fails mid-BB in a number of ways, including in simple ret void instructions under certain circumstances.

This dance of SelectionDAG (argument) -> FastISel (some instructions) -> SelectionDAG(remaining instructions) is lossy; in particular, Argument information lowering is cleared after that first SelectionDAG run.

Since swiftasync functions rely heavily on proper Argument lowering for debug information, this patch disables the use of FastISel in such functions.

(cherry picked from commit 83729e6)

Most (x86) swiftasync functions tend to use both SelectionDAGISel and
FastISel lowering:
* FastISel argument lowering can only handle C calling convention.
* FastISel fails mid-BB in a number of ways, including in simple `ret
void` instructions under certain circumstances.

This dance of SelectionDAG (argument) -> FastISel (some instructions) ->
SelectionDAG(remaining instructions) is lossy; in particular, Argument
information lowering is cleared after that first SelectionDAG run.

Since swiftasync functions rely heavily on proper Argument lowering for
debug information, this patch disables the use of FastISel in such
functions.

(cherry picked from commit 83729e6)
@felipepiovezan
Copy link
Author

@swift-ci please test

felipepiovezan added a commit to felipepiovezan/swift that referenced this pull request Nov 14, 2023
With swiftlang/llvm-project#7787 merged, we can now finally
renable these tests, as the x86 swift async handling should be properly
preserved by the x86 instruction selector.

These tests contained some incorrect checks from a time where it was expecting
an entry value even in the entry funclet of coroutines. For example, it had
this:
```
CHECK-NEXT: [[ASYNC_REG:DW_OP_.*], DW_OP_plus_uconst 0x10, DW_OP_plus_uconst 0x8,
DW_OP_deref
```

Which tries to find some "async reg", but really matches anything. It worked by
accident on arm, but not on x86. The correct check is simply:

```
CHECK-NOT: OP_entry_value
```
@felipepiovezan felipepiovezan merged commit 21398db into swiftlang:stable/20230725 Nov 14, 2023
@felipepiovezan felipepiovezan deleted the felipe/cherry_pick_disable_fast_isel2 branch November 14, 2023 21:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants