[libcxx-commits] [libcxx] [libcxx] Remove Redundant Reset in ~basic_string (PR #164718)

[Aiden Grossman via libcxx-commits]

boomanaiden154 wrote:

> Could you demonstrate the phase ordering problem please? Looking at the pipeline it seems to me like the dse is done before inlining, and the store is in fact eliminated in the example I gave, even though the destructor (which I annotated manually) did get inlined.

The example of just calling the destructor inside a function like in the example:
```c++
auto test(std::string& str) {
  str.~basic_string();
}
```

is not complex enough to demonstrate the problem. The store is still trivially dead at the end of the function and can be eliminated. The pass pipeline (taken from `opt <ll file> -passes="default<O3>" -print-pipeline-passes -disable-output`):
```
<snip>,cgscc(devirt<4>(inline,function-attrs<skip-non-recursive-function-attrs>,argpromotion,openmp-opt-cgscc,function<eager-inv;no-rerun>(dse,<snip>
```
Shows that DSE runs (quite a bit but not shown in the example) after we run inlining. Once this destructor gets inlined into a function of any reasonable size, Alias Analysis becomes very intractable to actually prove that the store does not alias anything, preventing DSE.

`dead_on_return` does not help here because it does not propagate anything through inlining. After inlining a function that marks a parameter as `dead_on_return`, we completely lose information about the lifetime of that parameter. That's probably possible to fix, but definitely high effort as it would require some reasonably big IR changes.

I can come up with an example walking through the pass pipeline demonstrating this, but it might be nontrivial which is why I haven't done so yet. 

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