[llvm] r334895 - CorrelatedValuePropagation: Preserve DT.

Michael Zolotukhin via llvm-commits llvm-commits at lists.llvm.org
Wed Jun 20 08:00:10 PDT 2018


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