[PATCH] D94576: [LoopVectorize] Guard verifyFunction with EXPENSIVE_CHECKS macro

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added a comment.

In D94576#3269996 <https://reviews.llvm.org/D94576#3269996>, @lebedev.ri wrote:

> In D94576#2611255 <https://reviews.llvm.org/D94576#2611255>, @tislam wrote:
>
>> Hi @lebedev.ri, did the second example work for you? Please let me know if you need any additional information.
>
> Sorry, this completely dropped off my radar.
> My comment is still the same, it would be good to to understand,
> is there some particular reason why verification is slow for that sample?
> Do you have a bisection as to when it became slow?
>
> Hiding verification under `EXPENSIVE_CHECKS` is somewhat problematic,
> it will effectively disable verification.

This verification gets expensive when there are a lot of loops that are processed in a function. It seems kind of odd to run the verification on the entire function on each loop that is processed. Since the cost of verification is something like `O(numloops x funcsize)`, it stands to reason that functions with many loops (which will also be quite large) will cause a huge cost to be paid for verification.
Simply modifying the vectorizer as:

  diff --git a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  index bef39a56d9f2..220977f91cf3 100644
  --- a/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  +++ b/llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  @@ -10243,7 +10243,6 @@ static bool processLoopInVPlanNativePath(
   
     // Mark the loop as already vectorized to avoid vectorizing again.
     Hints.setAlreadyVectorized();
  -  assert(!verifyFunction(*L->getHeader()->getParent(), &dbgs()));
     return true;
   }
   
  @@ -10672,7 +10671,6 @@ bool LoopVectorizePass::processLoop(Loop *L) {
       Hints.setAlreadyVectorized();
     }
   
  -  assert(!verifyFunction(*L->getHeader()->getParent(), &dbgs()));
     return true;
   }
   
  @@ -10738,6 +10736,8 @@ LoopVectorizeResult LoopVectorizePass::runImpl(
       Changed |= CFGChanged |= processLoop(L);
     }
   
  +  assert(!Changed || !verifyFunction(F, &dbgs()));
  +
     // Process each loop nest in the function.
     return LoopVectorizeResult(Changed, CFGChanged);
   }

cuts down the compile time by a factor of around 3.

The disadvantage of course is that in case of a broken function, the developer won't know which loop is transformed incorrectly. However, paying such a huge compile-time cost for something that `bugpoint` should easily help the developer narrow down in the rare case of a broken transformation, seems completely unnecessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94576