[PATCH] D46482: [ObjCARC] Prevent code motion into a catchswitch

[Shoaib Meenai via Phabricator via llvm-commits]

smeenai added inline comments.


================
Comment at: lib/Transforms/ObjCARC/ObjCARCOpts.cpp:1577
       const RRInfo &NewRetainRRI = It->second;
       KnownSafeTD &= NewRetainRRI.KnownSafe;
       for (Instruction *NewRetainRelease : NewRetainRRI.Calls) {
----------------
ahatanak wrote:
> If you want to set CFGHazardAfflicted inside BottomUpPtrState::HandlePotentialUse, perhaps you can read NewRetainRRI's CFGHazardAfflicted bit and set CFGHazardAfflicted here, just as it's done in line 1649?
Let me try that, thanks!


================
Comment at: lib/Transforms/ObjCARC/ObjCARCOpts.cpp:1699
+                CFGHazardAfflicted |= isa<CatchSwitchInst>(RIP);
               }
             }
----------------
rnk wrote:
> ahatanak wrote:
> > This is preventing releases from being inserted before a CatchSwitchInst. Is that correct? Do you have to prevent retains from being inserted before CatchSwitchInsts too somewhere, or we don't have to worry about it because it never happens?
> Yes, a catchswitch must be the first and last non-phi instruction in the BB. It's BB must be empty. It only exists to multiplex unwind edges from invokes.
I haven't seen retains be inserted before a catchswitch. I think that's because only the bottom-up analysis does the special treatment of invoke instructions, where they're analyzed as part of the successor basic blocks instead.


Repository:
  rL LLVM

https://reviews.llvm.org/D46482