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

[Coroutines][Swift] Remove replaceSwiftErrorOps while cloning #116292

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Commits on Nov 14, 2024

  1. [Coroutines][Swift] Remove replaceSwiftErrorOps while cloning

    I am confused by what replaceSwiftErrorOps is supposed to do and it
    doesn't seem to be well covered by lit-tests. At least in tree.
    
    The function appears to primarily operate on the original function,
    because it processes the SwiftErrorOps in Shape, collected from the
    unsplit function. However, it is called during cloning process of each
    resume function and from reading the code it seems to do something
    strange.
    
    After cloning the first resume funnction it may add Load and Store
    instructions to the original function. These would then appear in any
    subsequent resume functions that are cloned from the original, but not
    the one being processed. Instead an alloca will be created in the first
    resume function. After the first call to replaceSwiftErrorOps the
    SwiftErrorOps list is cleared so no other resume functions will get the
    alloca. Following this replaceSwiftErrorOps is called again after
    splitting but that would do nothing (right?).
    
    Removing the call within the Cloner::create() doesn't break any
    lit-tests. Can this be safely removed?
    
    I am looking at this because I am working on splitting. As explained in
    llvm#116285 I want to CloneAndPrune
    to create resume functions that only include the code they need and not
    the entire original function. However, this call causes
    coro-swifterror.ll to fail by:
    
    swifterror argument should come from an alloca or parameter
    ptr poison
      tail call void @maybeThrow(ptr swifterror poison)
    
    The swifterror argument is not correctly used in a few places in the IR
    and some how removing the replaceSwiftErrorOps call in Cloner::create()
    resolves the problem.
    tnowicki committed Nov 14, 2024
    Configuration menu
    Copy the full SHA
    13cca95 View commit details
    Browse the repository at this point in the history