[llvm] r334895 - CorrelatedValuePropagation: Preserve DT.
Michael Zolotukhin via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 20 09:00:51 PDT 2018
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