[llvm] [Coroutines] Refactor CoroShape::buildFrom for future use by ABI objects (PR #108623)

[Tyler Nowicki via llvm-commits]

================
@@ -50,15 +50,49 @@ enum class ABI {
 // Holds structural Coroutine Intrinsics for a particular function and other
 // values used during CoroSplit pass.
 struct LLVM_LIBRARY_VISIBILITY Shape {
-  CoroBeginInst *CoroBegin;
+  CoroBeginInst *CoroBegin = nullptr;
   SmallVector<AnyCoroEndInst *, 4> CoroEnds;
   SmallVector<CoroSizeInst *, 2> CoroSizes;
   SmallVector<CoroAlignInst *, 2> CoroAligns;
   SmallVector<AnyCoroSuspendInst *, 4> CoroSuspends;
-  SmallVector<CallInst *, 2> SwiftErrorOps;
   SmallVector<CoroAwaitSuspendInst *, 4> CoroAwaitSuspends;
   SmallVector<CallInst *, 2> SymmetricTransfers;
 
+  // Values invalidated by invalidateCoroutine() and cleanCoroutine()
+  SmallVector<CoroFrameInst *, 8> CoroFrames;
+  SmallVector<CoroSaveInst *, 2> UnusedCoroSaves;
----------------
TylerNowicki wrote:

I see you approved the change but I don't see a lgtm? Do you want me to make these nit changes in another patch?

About the nit changes, let me see if I have understood what you want.

1. Keep CoroShape::CoroFrames and CoroShape::UnusedCoroSaves
2. Declare CoroShape::ToBeInvalidatedCoroFrames and CoroShape::ToBeInvalidatedUnusedCoroSaves
3. In CoroShape::invalidateCoroutine() and CoroShape::cleanCoroutine() instead of erasing the Instructions, move them to CoroShape::ToBeInvalidated{CoroFrames,UnusedCoroSaves}
4. Add code to CoroShape::clear() or CoroShape::~CoroShape() to erase the Instructions in CoroShape::ToBeInvalidated{CoroFrames,UnusedCoroSaves}

Is that right?

Should we erase instructions in CoroShape::clear() or CoroShape::~CoroShape()?

Also I noticed that CoroShape::invalidateCoroutine() also erases CoroSuspends and CoroSaves (those that are used). Should we also delay erasing these until CoroShape::clear() or CoroShape::~CoroShape()?

https://github.com/llvm/llvm-project/pull/108623