[llvm] Reapply "[coro][CoroSplit] Use `llvm.lifetime.end` to compute putting objects on the frame vs the stack (#90265) (PR #91372)

Alan Zhao via llvm-commits llvm-commits at lists.llvm.org
Tue May 7 21:51:35 PDT 2024


alanzhao1 wrote:

> > > The code duplication is slightly not good. Can we try to mitigate such problems by some template techniques?
> > > For example, (may be):
> > > ```
> > > if constexpr (isMany) {
> > >     if (StopSet.contains(BB))
> > >       return true;
> > > } else {
> > >     if (BB == StopBB) // or BB == *StopSet.begin()
> > >       return true;
> > > }
> > > ```
> > 
> > 
> > Hmm...would making `StopBB`/`StopSet` a `std::variant<const BasicBlock *, const SmallPtrSetImpl<const BasicBlock *> *>` work?
> > EDIT: changed the second `std::variant` template parameter to a pointer as its template parameters can't be reference types.
> 
> Maybe it can be slightly better to have a isReachableImpl template function, where the type of `stop` can be a template, then we can judge the type directly in the `Impl` function. WDYT?

Personally, I'm leaning towards using `std::variant` because template type parameters don't have any information about the underlying type (LLVM hasn't migrated to C++20 yet, so we don't have concepts), so the only feedback we have is that the code doesn't compile because a type doesn't have a method. A variant parameter however does retain type information, and the code would be virtually identical to what you propose - e.g.

```cpp
const auto StopBB** = std::get_if<const BasicBlock *>(StopBBOrSet);
const auto StopSet** = std::get_if<const SmallPtrSetImpl<const BasicBlock *> *>(StopBBOrSet);
if (StopSet) {
    if ((*StopSet)->contains(BB))
      return true;
} else {
    if (BB == *StopBB)
      return true;
}
```

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


More information about the llvm-commits mailing list