[llvm] r334895 - CorrelatedValuePropagation: Preserve DT.
Benjamin Kramer via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 20 09:04:19 PDT 2018
In JumpThreading. I don't think CVT is the problem here, it just
doesn't trigger a dominator recompute anymore.
On Wed, Jun 20, 2018 at 6:00 PM Michael Zolotukhin
<mzolotukhin at apple.com> wrote:
>
> Are you talking about flushing DDT in JT or in CVT?
>
> > On Jun 20, 2018, at 8:31 AM, Benjamin Kramer <benny.kra at gmail.com> wrote:
> >
> > Sadly I can't share it and I don't know how to reduce it. Verification
> > doesn't catch it either. What works is flushing DeferredDominators
> > after we delete a block, which makes me think that there's something
> > wrong with the way we handle dead blocks.
> > On Wed, Jun 20, 2018 at 5:00 PM Michael Zolotukhin
> > <mzolotukhin at apple.com> wrote:
> >>
> >> Hmm, that’s interesting. Could you please share your test-case so that I can poke at it too? Another thing to try would be to add explicit DT verification after JT - if recomputation helps, then the existing DT should be incorrect at that point.
> >>
> >> Michael
> >>
> >>> On Jun 20, 2018, at 6:11 AM, Benjamin Kramer <benny.kra at gmail.com> wrote:
> >>>
> >>> 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