Fix another use-after-free in SILCombine

[meg-gupta]

swift::endLifetimeAtFrontier also needs to use swift::emitDestroyOperation and delete instructions via callbacks that
can correctly remove it from the worklist that SILCombine maintains.

[meg-gupta]

@swift-ci smoke test

[atrick]

I can't make sense of this design. Why is the builder deleting anything? And why would we pass around callbacks in addition to a builder? The builder is only supposed to create instructions (or find an existing instruction to use), and it keeps a track of any new instructions it creates. Basic SIL utilities, which often take a builder, shouldn't delete anything either, and shouldn't need to take any callbacks. If SILCombine needs to know about new instructions, it can get that from the builder's context.

[meg-gupta]

@atrick this is a follow-up of #34145 which has more context. SILBuilder has several api's that deletes instructions. See SILBuilder::emitStrongReleaseAndFold, SILBuilder::emitReleaseValueAndFold. I think these were added several years back before a powerful ARC optimizer. My first attempt at fixing this issue was to get rid of "folding" in the SILBuilder apis. Several tests need to be updated in -Onone because guaranteed opts at -Onone does not catch these "folding" patterns.

[meg-gupta]

SILCombine needs to know about the deleted instructions, and some of these apis deceivingly deletes instructions. The right thing would still be to get rid of "folding" by fixing all Onone tests or improving guaranteed arc opts at -Onone which did not look straight-forward at quick glance. This use-after-free is blocking another PR.

[atrick]

@meg-gupta @eeckstein I just want to make it clear that none of these utility APIs should be taking callbacks and the builder should never erase instructions. It's fine for the builder to "fold" a potential new instruction into an existing one. Any other peepholes that the builder does should be in a separate cleanup routine that's called by the pass, given callbacks if needed, given the same BuilderContext, and driven by the list of new instructions.

[meg-gupta]

@swift-ci smoke test OS X Platform

[gottesmm]

@atrick @meg-gupta my two thoughts are that:

1. We should consider just removing these folding APIs in the builders. If we need someone to go through the tests, I have a little time I would be happy to do it since I have wanted to do it for 5 years = p.
2. This PR needs a test.

[atrick]

@gottesmm I looked at the issue with @meg-gupta. The SILBuilder optimizations that delete instructions can be canonicalizations, which means we shouldn't need to update any tests, unless we end up optimizing even more in some cases. That can be a follow up.

[eeckstein]

I agree, that the SILBuilder APIs which delete instructions are pretty bad and we should remove them.
Until we do that, we have to live with it.

This PR lgtm.
@meg-gupta can you add a test case?

[meg-gupta]

@swift-ci smoke test

[meg-gupta]

@swift-ci smoke test Linux Platform