SLP/Loop vectorizer pass ordering

[Arnold Schwaighofer]

> On Oct 29, 2014, at 4:12 PM, Michael Zolotukhin <mzolotukhin at apple.com> wrote:
> 
> Okay, seemingly it’s just a bug in the vectorizer: it updates DT incorrectly. Specifically, this line is incorrect:
>   DT->changeImmediateDominator(LoopExitBlock, LoopMiddleBlock);
> 
> The correct dominator for LoopExitBlock in this case would be LoopBypassBlocks[0] (not LoopMiddleBlock). I’ll send a patch soon.

Yes that sounds right - after introducing the overflow check at the beginning of the loop skeleton the “LoopMiddleBlock” no longer dominates.

       [ ] <-- Back-edge taken count overflow check. <=== This is LoopBypassBlocks[0]
    /   |
   /    v
  |    [ ] <-- vector loop bypass (may consist of multiple blocks).
  |  /  |
  | /   v
  ||   [ ]     <-- vector pre header.
  ||    |
  ||    v
  ||   [  ] \
  ||   [  ]_|   <-- vector loop.
  ||    |
  | \   v
  |   >[ ]   <--- middle-block.
  |  /  |
  | /   v
  -|- >[ ]     <--- new preheader.
   |    |
   |    v
   |   [ ] \
   |   [ ]_|   <-- old scalar loop to handle remainder.
    \   |
     \  v
      >[ ]     <-- exit block.
   ...
   */

If you want to verify that a dominator tree is still valid you can call DomTree->verifyDomTree() which will rebuild the dom tree from scratch and compare it to the current dom tree.

It is surprising to me that this did not get caught by verifyAnalysis …

Dominators.cpp:

// Always verify dominfo if expensive checking is enabled.
#ifdef XDEBUG
static bool VerifyDomInfo = true;
#else
static bool VerifyDomInfo = false;
#endif
static cl::opt<bool,true>
VerifyDomInfoX("verify-dom-info", cl::location(VerifyDomInfo),
               cl::desc("Verify dominator info (time consuming)”))
…
void DominatorTree::verifyDomTree() const {
  if (!VerifyDomInfo)
    return;
…

Ah, there is my answer I guess, we don’t have an XDEBUG bot.

Thanks,
Arnold