[PATCH] D56079: Fix invalid test in Guard Widening

Max Kazantsev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 29 03:45:52 PST 2019


mkazantsev added a comment.

> I don't remember what was this situation, but there was a case where inner was chosen a root

I've refreshed it in my mind now. The current algorithm of choise of non-trivial root will choose `outer` as the furthermost block. And therefore the 2nd guard will post-dominate the 1st one. If we turn the guards into a form of widenable branches, it will look like:

  inner:
    br guaded1, deopt1
  guarded1:
    br inner, outer
  outer:
    br guarded2, deopt2
  guarded2: 
    br inner

This graph has a trivial root `deopt2`. Now block `outer` does not post-dominate block `inner` because inner has a branch to deopt1. So we can no longer widen.

So in fact, this test is only checking fact that PDT *happens* to choose furthest block as its non-trivial root and *happens* to ignore the implicit control flow of guards. Both these facts may in theory change. So maybe UB was a too harsh word here, but the fact that we can do widening here stands on two highly flacky facts.

Besides, this test's comment does not correspond the IR.


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

https://reviews.llvm.org/D56079





More information about the llvm-commits mailing list