[AMDGPU] Use verify<domtree> instead of intra-pass asserts.

[pravinjagtap]

Summary

Verifying dominator tree is expensive using intra-pass
asserts. Asserts added during D147408 are
increasing the build time of libc significantly. This change
does the verification after the atomic optimizer pass
and should fix the regression reported in D153232.

[arsenm]

Do we really need this intra-pass verification? With -verify-dom-info the DominatorTree verification should fail after the pass like normal

llvm/lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp

799

assert(!VerifyDomInfo || ....

[foad]

If -verify-dom-info is used then the normal analysis verification should catch this after this pass has run, as mentioned here: https://reviews.llvm.org/D147408#inline-1471284

So I do not think you need any explicit calls to DominatorTree::verify

[pravinjagtap]

In D153261#4432298, @foad wrote:

If -verify-dom-info is used then the normal analysis verification should catch this after this pass has run, as mentioned here: https://reviews.llvm.org/D147408#inline-1471284

So I do not think you need any explicit calls to DominatorTree::verify

Unfortunately No. The -verify-dom-info is not catching the un-updated dom tree.

[arsenm]

In D153261#4432462, @pravinjagtap wrote:

In D153261#4432298, @foad wrote:

If -verify-dom-info is used then the normal analysis verification should catch this after this pass has run, as mentioned here: https://reviews.llvm.org/D147408#inline-1471284

So I do not think you need any explicit calls to DominatorTree::verify

Unfortunately No. The -verify-dom-info is not catching the un-updated dom tree.

Is this just the analysis was dropped because nothing after it was using it problem? If you run the pass twice with -passes do you see the error?

[pravinjagtap]

In D153261#4432463, @arsenm wrote:

In D153261#4432462, @pravinjagtap wrote:

In D153261#4432298, @foad wrote:

If -verify-dom-info is used then the normal analysis verification should catch this after this pass has run, as mentioned here: https://reviews.llvm.org/D147408#inline-1471284

So I do not think you need any explicit calls to DominatorTree::verify

Unfortunately No. The -verify-dom-info is not catching the un-updated dom tree.

Is this just the analysis was dropped because nothing after it was using it problem? If you run the pass twice with -passes do you see the error?

Understood why dom-tree verifier was not catching the un-updated DT. After IR transformations in AtomicOptimizer Analyses results are invalidated (when IR is transformed) and verifier re-computes the dom-tree freshly.

[pravinjagtap]

Removed the asserts and added dom-tree verification test

[yassingh]

Why aren't we preserving dom-tree if were are updating it?

[arsenm]

In D153261#4434599, @yassingh wrote:

Why aren't we preserving dom-tree if were are updating it?

It looks like it is reported as preserved in the old PM, but not the new

llvm/lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp

150

This should be fixed to report the dominator tree is updated

[arsenm]

LGTM. Please fix the missing new PM preserves dominator tree separately

llvm/lib/Target/AMDGPU/AMDGPUAtomicOptimizer.cpp

PreservedAnalyses AMDGPUAtomicOptimizerPass::run(Function &F,
   DomTreeUpdater DTU(&AM.getResult<DominatorTreeAnalysis>(F),
                      DomTreeUpdater::UpdateStrategy::Lazy);
   const GCNSubtarget *ST = &TM.getSubtarget<GCNSubtarget>(F);
 
   bool IsPixelShader = F.getCallingConv() == CallingConv::AMDGPU_PS;
 
   return AMDGPUAtomicOptimizerImpl(UA, DL, DTU, ST, IsPixelShader, ScanImpl)
                  .run(F)
              ? PreservedAnalyses::none()
              : PreservedAnalyses::all();
 }
 
 bool AMDGPUAtomicOptimizerImpl::run(Function &F) {
 
   visit(F);
 
   const bool Changed = !ToReplace.empty();
void AMDGPUAtomicOptimizerImpl::optimizeAtomic(Instruction &I,
   // We need to introduce some new control flow to force a single lane to be
   // active. We do this by splitting I's basic block at I, and introducing the
   // new block such that:
   // entry --> single_lane -\
   //       \------------------> exit
   Instruction *const SingleLaneTerminator =
       SplitBlockAndInsertIfThen(Cond, &I, false, nullptr, &DTU, nullptr);
 
-  // Control flow is changed here after splitting I's block
-  assert(DTU.getDomTree().verify(DominatorTree::VerificationLevel::Full));
-
   // At this point, we have split the I's block to allow one lane in wavefront
   // to update the precomputed reduced value. Also, completed the codegen for
   // new control flow i.e. iterative loop which perform reduction and scan using
   // ComputeLoop and ComputeEnd.
   // For the new control flow, we need to move branch instruction i.e.
   // terminator created during SplitBlockAndInsertIfThen from I's block to
   // ComputeEnd block. We also need to set up predecessor to next block when
   // single lane done updating the final reduced value.
if (ValDivergent && ScanImpl == ScanOptions::Iterative) {
     B.SetInsertPoint(EntryBB);
     B.CreateBr(ComputeLoop);
 
     // Update the dominator tree for new control flow.
     DTU.applyUpdates(
         {{DominatorTree::Insert, EntryBB, ComputeLoop},
          {DominatorTree::Insert, ComputeLoop, ComputeEnd},
          {DominatorTree::Delete, EntryBB, SingleLaneTerminator->getParent()}});
-    assert(DTU.getDomTree().verify(DominatorTree::VerificationLevel::Full));
 
     Predecessor = ComputeEnd;
   } else {
     Predecessor = EntryBB;
   }
   // Move the IR builder into single_lane next.
   B.SetInsertPoint(SingleLaneTerminator);
 
   // Clone the original atomic operation into single lane, replacing the

llvm/test/CodeGen/AMDGPU/global_atomics_iterative_scan.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN:  opt -S -mtriple=amdgcn-- -amdgpu-atomic-optimizer-strategy=Iterative -passes=amdgpu-atomic-optimizer %s | FileCheck -check-prefix=IR %s
+; RUN:  opt -S -mtriple=amdgcn-- -amdgpu-atomic-optimizer-strategy=Iterative -passes='amdgpu-atomic-optimizer,verify<domtree>' %s | FileCheck -check-prefix=IR %s
 
 define amdgpu_kernel void @uniform_value(ptr addrspace(1) , ptr addrspace(1) %val) #0 {
 ; IR-LABEL: @uniform_value(
 ; IR-NEXT:  entry:
 ; IR-NEXT:    [[UNIFORM_VALUE_KERNARG_SEGMENT:%.*]] = call nonnull align 16 dereferenceable(52) ptr addrspace(4) @llvm.amdgcn.kernarg.segment.ptr()
 ; IR-NEXT:    [[OUT_KERNARG_OFFSET:%.*]] = getelementptr inbounds i8, ptr addrspace(4) [[UNIFORM_VALUE_KERNARG_SEGMENT]], i64 36
 ; IR-NEXT:    [[LOADED_OUT_KERNARG_OFFSET:%.*]] = load <2 x i64>, ptr addrspace(4) [[OUT_KERNARG_OFFSET]], align 4
 ; IR-NEXT:    [[OUT_LOAD1:%.*]] = extractelement <2 x i64> [[LOADED_OUT_KERNARG_OFFSET]], i32 0