[all-commits] [llvm/llvm-project] 87235f: [NFC] CoroElide: Refactor `Lowerer` into `CoroIdEl...

[Yuxuan Chen via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 87235fa9af06b639d7b4b1eb7ac5840fb56cc569
      https://github.com/llvm/llvm-project/commit/87235fa9af06b639d7b4b1eb7ac5840fb56cc569
  Author: Yuxuan Chen <yuxuanchen1997 at outlook.com>
  Date:   2024-05-09 (Thu, 09 May 2024)

  Changed paths:
    M llvm/lib/Transforms/Coroutines/CoroElide.cpp

  Log Message:
  -----------
  [NFC] CoroElide: Refactor `Lowerer` into `CoroIdElider` (#91539)

This patch contains no functional changes. 

The main goal of this patch is to get better clarity out of the code, to
make intentions and assumptions clear.

One major design problem I had in the past were `Lowerer`. It previously
inherited from `coro::LowererBase` but it doesn't use any of the fields
or methods from `LowererBase`. It might be an artifact leftover from
previous designs of this code.

Furthermore, we should clarify that although one such instance is bound
to the function, `Lowerer` was dedicated to one `CoroId` instruction at
a time. We rely on a sequence of fragile constructs like
`CoroBegins.clear(); DestroyAddr.clear()`. This doesn't help understand
the code.

What's worse is that we have confusing calls like
`elideHeapAllocations(CoroId->getFunction(), ...` and it might get
confused with `CoroId->getCoroutine()`.

The new structure intends to make it clear that we always operate on one
`CoroId` at a time, which may have multiple `CoroBegin`s. Such structure
doesn't rely on frequent `.clear()` that's prone to miss.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications