[PATCH] D100640: [SimplifyCFG] Skip dbg intrinsics when checking for branch-only BBs.

[Florian Hahn via Phabricator via llvm-commits]

fhahn added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SimplifyCFG.cpp:1442
       return false;
-    if (!I1->isTerminator())
+    if (!I2NonDbg->isTerminator())
       return false;
----------------
lebedev.ri wrote:
> fhahn wrote:
> > uabelho wrote:
> > > Since I1NonDbg and I2NonDbg should be identical here I guess it doesn't really matter, but anyway: Wouldn't it make the patch look a little bit simpler if we used I1NonDbg here rather than changing from I1 to I2NonDbg?
> > That's indeed better, thanks. Updated!
> I think this isn't quite right. Sure, they are free to host. But now we will simply always drop them?
> I think we should still hoist all identical debug info, and skip the rest of the debuginfo.
I think we should now handle dbg intrinsics the same as if hoisting was enabled, because we just delegate the hoisting to the existing code. Here we just skip debug intrinsics for the check whether the only non-debug instruction is a branch. The loop should hoist any debug intrinsics if it can, because we do not change `I1` and `I2`.

In `hoist_with_debug2` the `dbg.value` calls are hoisted. They are dropped for `hoist_with_debug3_pr49982`, but I think not by the hoisting cod but another transform in SimplifyCFG, as they are also dropped without this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100640