Keep CodeGenPrepare from preserving the dominator tree

[george.burgess.iv]

Summary

CodeGenPrepare may invalidate the domtree. At the moment, there's no way to selectively preserve passes, so we need to be conservative and never preserve it.

[george.burgess.iv]

Note: D16984 incorporates this into a slightly larger/more complete change. I'll abandon this when that goes in.

[jmolloy]

LGTM.

As CGP is the last pass run before SDAG, how exactly did you find this bug out of interest?

James

[george.burgess.iv]

LGTM.

Thank you! :) Like said, D16984 seems to absorb this change, so I'll commit this if we decide to abandon that.

As CGP is the last pass run before SDAG, how exactly did you find this bug out of interest?

I wrote a script that more or less appends -basicaa -verify-memoryssa to all opt runlines in our test suite. This caused a CGP test that was originally just running -loop-unroll -codegenprepare to segfault, because the domtree was pointing to deleted basic blocks when it was handed to MemorySSA.

lib/CodeGen/CodeGenPrepare.cpp

explicit CodeGenPrepare(const TargetMachine *TM = nullptr)
         : FunctionPass(ID), TM(TM), TLI(nullptr), TTI(nullptr), DL(nullptr) {
         initializeCodeGenPreparePass(*PassRegistry::getPassRegistry());
       }
     bool runOnFunction(Function &F) override;
 
     const char *getPassName() const override { return "CodeGen Prepare"; }
 
     void getAnalysisUsage(AnalysisUsage &AU) const override {
-      AU.addPreserved<DominatorTreeWrapperPass>();
+      // FIXME: When we can selectively preserve passes, preserve the domtree.
       AU.addRequired<TargetLibraryInfoWrapperPass>();
       AU.addRequired<TargetTransformInfoWrapperPass>();
     }
 
   private:
     bool eliminateFallThrough(Function &F);
     bool eliminateMostlyEmptyBlocks(Function &F);
     bool canMergeBlocks(const BasicBlock *BB, const BasicBlock *DestBB) const;
bool CodeGenPrepare::optimizeBlock(BasicBlock &BB, bool& ModifiedDT) {
   }
 
   bool MadeBitReverse = true;
   while (TLI && MadeBitReverse) {
     MadeBitReverse = false;
     for (auto &I : reverse(BB)) {
       if (makeBitReverse(I, *DL, *TLI)) {
         MadeBitReverse = MadeChange = true;
+        ModifiedDT = true;
         break;
       }
     }
   }
   MadeChange |= dupRetToEnableTailCallOpts(&BB);
 
   return MadeChange;
 }

test/Transforms/CodeGenPrepare/dom-tree.ll

+; RUN: opt -S -loop-unroll -codegenprepare < %s -domtree -analyze | FileCheck %s
+;
+; Checks that the dom tree is properly invalidated after an operation that will
+; invalidate it in CodeGenPrepare. If the domtree isn't properly invalidated,
+; this will likely segfault, or print badref.
+
+; CHECK-NOT: <badref>
+
+target datalayout = "e-m:e-p:32:32-i64:64-v128:64:128-a:0:32-n32-S64"
+target triple = "armv7--linux-gnueabihf"
+
+define i32 @f(i32 %a) #0 {
+entry:
+  br label %for.body
+
+for.cond.cleanup:
+  ret i32 %or
+
+for.body:
+  %i.08 = phi i32 [ 0, %entry ], [ %inc, %for.body ]
+  %b.07 = phi i32 [ 0, %entry ], [ %or, %for.body ]
+  %shr = lshr i32 %a, %i.08
+  %and = and i32 %shr, 1
+  %sub = sub nuw nsw i32 31, %i.08
+  %shl = shl i32 %and, %sub
+  %or = or i32 %shl, %b.07
+  %inc = add nuw nsw i32 %i.08, 1
+  %exitcond = icmp eq i32 %inc, 32
+  br i1 %exitcond, label %for.cond.cleanup, label %for.body, !llvm.loop !3
+}
+
+attributes #0 = { norecurse nounwind readnone "disable-tail-calls"="false" "less-precise-fpmad"="false" "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" "no-infs-fp-math"="false" "no-nans-fp-math"="false" "stack-protector-buffer-size"="8" "target-cpu"="cortex-a8" "target-features"="+dsp,+neon,+vfp3" "unsafe-fp-math"="false" "use-soft-float"="false" }
+
+!llvm.module.flags = !{!0, !1}
+!llvm.ident = !{!2}
+
+!0 = !{i32 1, !"wchar_size", i32 4}
+!1 = !{i32 1, !"min_enum_size", i32 4}
+!2 = !{!"clang version 3.8.0 (http://llvm.org/git/clang.git b7441a0f42c43a8eea9e3e706be187252db747fa)"}
+!3 = distinct !{!3, !4}
+!4 = !{!"llvm.loop.unroll.full"}