[PATCH] D49974: [GuardWidening] Widen guards with conditions of frequently taken dominated branches

[Fedor Sergeev via Phabricator via llvm-commits]

fedor.sergeev added inline comments.


================
Comment at: lib/Transforms/Scalar/GuardWidening.cpp:408
+  // Only erase guard intrinsics. Do nothing about branches.
+  if (IntrinsicInst *GI = dyn_cast<IntrinsicInst>(GuardInst))
+    GI->eraseFromParent();
----------------
mkazantsev wrote:
> reames wrote:
> > mkazantsev wrote:
> > > reames wrote:
> > > > Better to move this check into the caller.
> > > No, it is here because we should not remove `br` instructions. We don't want the caller to know about possible types of guards.
> > I was suggesting that we only call this function for guards, not for branches.  Calling a function called "eliminateGuard" on a branch seems a bit odd.
> Here (and in other places like "getGuardCondition") we assume that the word "guard" has a wider definition than `llvm.experimental.guard` intrinsic. A branch can be a guard as well in some circumstances.
Technically this is widening of a guard with a branch/guard condition.
And in this patch we are not treating dominated condition exactly as a guard.
And elimination happens for branches and guards differently.
So I definitely see a merit in what Philip suggests.

Alternatively you might call it eliminateBranchConditionOrGuard ;)


https://reviews.llvm.org/D49974