[PATCH] D129370: [SimplifyCFG] Allow SimplifyCFG hoisting to skip over non-matching instructions

Fri Apr 28 03:17:40 PDT 2023

chill marked 2 inline comments as done.
chill added inline comments.

================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:1584
+        if (C1->isMustTailCall() != C2->isMustTailCall())
+          return Changed;
+
----------------
foad wrote:
> chill wrote:
> > chill wrote:
> > > foad wrote:
> > > > Leaving these `return` instructions in the `isIdenticalToWhenDefined` case seems wrong, since it means that instructions that are identical but fail this check are treated more pessimistically than instructions that are not identical.
> > > You mean ,in that case we should enter that part where we skip over those calls, increment
> > > `NumSkipped` and continue (i.e. the part that starts at  line 1638 below)?
> > > 
> > > Yeah, makes sense.
> > OTOH,  a `musttail` call can only be followed by a `ret` or a `bitcast`.
> > 
> > I guess I don't understand what we are missing here ...
> I didn't really mean this specific `return`, I meant all of the `return`s from here down to line 1595. D149365 fixes them all and shows a case where it improves handling of convergent calls.
Yeah, I saw the other patch. Thanks!

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129370