[llvm] r334895 - CorrelatedValuePropagation: Preserve DT.

Benjamin Kramer via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 20 06:11:26 PDT 2018


I'm seeing non-determinism after this change. Since this change is
fairly simple it's probably coming from earlier in the pipeline,
likely candidate being JumpThreading. Recalculating DT before
JumpThreading doesn't fix it, but recalculating after JumpThreading
removes the non-determinism.

My test case is coming out of tfcompile and is huge and scary, any
ideas on how to track this down?
On Sat, Jun 16, 2018 at 9:01 PM Michael Zolotukhin via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
>
> Author: mzolotukhin
> Date: Sat Jun 16 11:57:31 2018
> New Revision: 334895
>
> URL: http://llvm.org/viewvc/llvm-project?rev=334895&view=rev
> Log:
> CorrelatedValuePropagation: Preserve DT.
>
> Summary:
> We only modify CFG in a couple of places, and we can preserve DT there
> with a little effort.
>
> Reviewers: davide, vsk
>
> Subscribers: hiraditya, llvm-commits
>
> Differential Revision: https://reviews.llvm.org/D48059
>
> Modified:
>     llvm/trunk/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
>     llvm/trunk/test/Other/opt-O2-pipeline.ll
>     llvm/trunk/test/Other/opt-O3-pipeline.ll
>     llvm/trunk/test/Other/opt-Os-pipeline.ll
>
> Modified: llvm/trunk/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp?rev=334895&r1=334894&r2=334895&view=diff
> ==============================================================================
> --- llvm/trunk/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp (original)
> +++ llvm/trunk/lib/Transforms/Scalar/CorrelatedValuePropagation.cpp Sat Jun 16 11:57:31 2018
> @@ -82,6 +82,7 @@ namespace {
>        AU.addRequired<DominatorTreeWrapperPass>();
>        AU.addRequired<LazyValueInfoWrapperPass>();
>        AU.addPreserved<GlobalsAAWrapperPass>();
> +      AU.addPreserved<DominatorTreeWrapperPass>();
>      }
>    };
>
> @@ -306,7 +307,7 @@ static bool processCmp(CmpInst *C, LazyV
>  /// that cannot fire no matter what the incoming edge can safely be removed. If
>  /// a case fires on every incoming edge then the entire switch can be removed
>  /// and replaced with a branch to the case destination.
> -static bool processSwitch(SwitchInst *SI, LazyValueInfo *LVI) {
> +static bool processSwitch(SwitchInst *SI, LazyValueInfo *LVI, DominatorTree *DT) {
>    Value *Cond = SI->getCondition();
>    BasicBlock *BB = SI->getParent();
>
> @@ -321,6 +322,10 @@ static bool processSwitch(SwitchInst *SI
>
>    // Analyse each switch case in turn.
>    bool Changed = false;
> +  DenseMap<BasicBlock*, int> SuccessorsCount;
> +  for (auto *Succ : successors(BB))
> +    SuccessorsCount[Succ]++;
> +
>    for (auto CI = SI->case_begin(), CE = SI->case_end(); CI != CE;) {
>      ConstantInt *Case = CI->getCaseValue();
>
> @@ -355,7 +360,8 @@ static bool processSwitch(SwitchInst *SI
>
>      if (State == LazyValueInfo::False) {
>        // This case never fires - remove it.
> -      CI->getCaseSuccessor()->removePredecessor(BB);
> +      BasicBlock *Succ = CI->getCaseSuccessor();
> +      Succ->removePredecessor(BB);
>        CI = SI->removeCase(CI);
>        CE = SI->case_end();
>
> @@ -365,6 +371,8 @@ static bool processSwitch(SwitchInst *SI
>
>        ++NumDeadCases;
>        Changed = true;
> +      if (--SuccessorsCount[Succ] == 0)
> +        DT->deleteEdge(BB, Succ);
>        continue;
>      }
>      if (State == LazyValueInfo::True) {
> @@ -381,10 +389,14 @@ static bool processSwitch(SwitchInst *SI
>      ++CI;
>    }
>
> -  if (Changed)
> +  if (Changed) {
>      // If the switch has been simplified to the point where it can be replaced
>      // by a branch then do so now.
> -    ConstantFoldTerminator(BB);
> +    DeferredDominance DDT(*DT);
> +    ConstantFoldTerminator(BB, /*DeleteDeadConditions = */ false,
> +                           /*TLI = */ nullptr, &DDT);
> +    DDT.flush();
> +  }
>
>    return Changed;
>  }
> @@ -722,7 +734,7 @@ static bool runImpl(Function &F, LazyVal
>      Instruction *Term = BB->getTerminator();
>      switch (Term->getOpcode()) {
>      case Instruction::Switch:
> -      BBChanged |= processSwitch(cast<SwitchInst>(Term), LVI);
> +      BBChanged |= processSwitch(cast<SwitchInst>(Term), LVI, DT);
>        break;
>      case Instruction::Ret: {
>        auto *RI = cast<ReturnInst>(Term);
> @@ -767,5 +779,6 @@ CorrelatedValuePropagationPass::run(Func
>      return PreservedAnalyses::all();
>    PreservedAnalyses PA;
>    PA.preserve<GlobalsAA>();
> +  PA.preserve<DominatorTreeAnalysis>();
>    return PA;
>  }
>
> Modified: llvm/trunk/test/Other/opt-O2-pipeline.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/opt-O2-pipeline.ll?rev=334895&r1=334894&r2=334895&view=diff
> ==============================================================================
> --- llvm/trunk/test/Other/opt-O2-pipeline.ll (original)
> +++ llvm/trunk/test/Other/opt-O2-pipeline.ll Sat Jun 16 11:57:31 2018
> @@ -145,7 +145,6 @@
>  ; CHECK-NEXT:         Lazy Value Information Analysis
>  ; CHECK-NEXT:         Jump Threading
>  ; CHECK-NEXT:         Value Propagation
> -; CHECK-NEXT:         Dominator Tree Construction
>  ; CHECK-NEXT:         Basic Alias Analysis (stateless AA impl)
>  ; CHECK-NEXT:         Function Alias Analysis Results
>  ; CHECK-NEXT:         Memory Dependence Analysis
>
> Modified: llvm/trunk/test/Other/opt-O3-pipeline.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/opt-O3-pipeline.ll?rev=334895&r1=334894&r2=334895&view=diff
> ==============================================================================
> --- llvm/trunk/test/Other/opt-O3-pipeline.ll (original)
> +++ llvm/trunk/test/Other/opt-O3-pipeline.ll Sat Jun 16 11:57:31 2018
> @@ -149,7 +149,6 @@
>  ; CHECK-NEXT:         Lazy Value Information Analysis
>  ; CHECK-NEXT:         Jump Threading
>  ; CHECK-NEXT:         Value Propagation
> -; CHECK-NEXT:         Dominator Tree Construction
>  ; CHECK-NEXT:         Basic Alias Analysis (stateless AA impl)
>  ; CHECK-NEXT:         Function Alias Analysis Results
>  ; CHECK-NEXT:         Memory Dependence Analysis
>
> Modified: llvm/trunk/test/Other/opt-Os-pipeline.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Other/opt-Os-pipeline.ll?rev=334895&r1=334894&r2=334895&view=diff
> ==============================================================================
> --- llvm/trunk/test/Other/opt-Os-pipeline.ll (original)
> +++ llvm/trunk/test/Other/opt-Os-pipeline.ll Sat Jun 16 11:57:31 2018
> @@ -131,7 +131,6 @@
>  ; CHECK-NEXT:         Lazy Value Information Analysis
>  ; CHECK-NEXT:         Jump Threading
>  ; CHECK-NEXT:         Value Propagation
> -; CHECK-NEXT:         Dominator Tree Construction
>  ; CHECK-NEXT:         Basic Alias Analysis (stateless AA impl)
>  ; CHECK-NEXT:         Function Alias Analysis Results
>  ; CHECK-NEXT:         Memory Dependence Analysis
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


More information about the llvm-commits mailing list