[PATCH] D113105: [InstCombine] Extend stacksave/restore elimination

[Roman Lebedev via Phabricator via llvm-commits]

lebedev.ri added inline comments.


================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineCalls.cpp:1813-1814
+        for (++BI; &*BI != II; ++BI) {
+          if (Classify(&*BI) == ClassifyResult::None)
+            continue;
+
----------------
nextsilicon-itay-bookstein wrote:
> lebedev.ri wrote:
> > nextsilicon-itay-bookstein wrote:
> > > lebedev.ri wrote:
> > > > What about stackrestore from the same stacksave?
> > > Then it will have been eliminated when it was the subject of the enclosing switch statement, before this one was. I will add an appropriate comment.
> > I'm mainly highlighting that it might not have already happened because of the processing order.
> > It would be really hard to come up with an appropriate test manually,
> > but i suspect it is possible.
> I see. So it's a matter of not not deferring the work to the next InstCombine iteration (or missing the opportunity if it's we're at the iteration cap). Would you prefer that I replace the comment with the check?
> So it's a matter of not not deferring the work to the next InstCombine iteration (or missing the opportunity if it's we're at the iteration cap).

Right.

> Would you prefer that I replace the comment with the check?

I think that would be better, yes.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113105/new/

https://reviews.llvm.org/D113105