[llvm] r211433 - SCEVExpander: Fold constant PHIs harder. The logic below only understands proper IVs.

[Benjamin Kramer]

On 22.06.2014, at 08:55, Tobias Grosser <tobias at grosser.es> wrote:

> On 21/06/2014 13:47, Benjamin Kramer wrote:
>> Author: d0k
>> Date: Sat Jun 21 06:47:18 2014
>> New Revision: 211433
>> 
>> URL: http://llvm.org/viewvc/llvm-project?rev=211433&view=rev
>> Log:
>> SCEVExpander: Fold constant PHIs harder. The logic below only understands proper IVs.
>> 
>> PR20093.
>> 
>> Added:
>>     llvm/trunk/test/Transforms/IndVarSimplify/2014-06-21-congruent-constant.ll
>> Modified:
>>     llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp
>> 
>> Modified: llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp?rev=211433&r1=211432&r2=211433&view=diff
>> ==============================================================================
>> --- llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp (original)
>> +++ llvm/trunk/lib/Analysis/ScalarEvolutionExpander.cpp Sat Jun 21 06:47:18 2014
>> @@ -16,6 +16,7 @@
>>  #include "llvm/Analysis/ScalarEvolutionExpander.h"
>>  #include "llvm/ADT/STLExtras.h"
>>  #include "llvm/ADT/SmallSet.h"
>> +#include "llvm/Analysis/InstructionSimplify.h"
>>  #include "llvm/Analysis/LoopInfo.h"
>>  #include "llvm/Analysis/TargetTransformInfo.h"
>>  #include "llvm/IR/DataLayout.h"
>> @@ -1706,7 +1707,7 @@ unsigned SCEVExpander::replaceCongruentI
>> 
>>      // Fold constant phis. They may be congruent to other constant phis and
>>      // would confuse the logic below that expects proper IVs.
>> -    if (Value *V = Phi->hasConstantValue()) {
>> +    if (Value *V = SimplifyInstruction(Phi, SE.DL, SE.TLI, SE.DT)) {
>>        Phi->replaceAllUsesWith(V);
>>        DeadInsts.push_back(Phi);
>>        ++NumElim;
>> 
>> Added: llvm/trunk/test/Transforms/IndVarSimplify/2014-06-21-congruent-constant.ll
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/IndVarSimplify/2014-06-21-congruent-constant.ll?rev=211433&view=auto
>> ==============================================================================
>> --- llvm/trunk/test/Transforms/IndVarSimplify/2014-06-21-congruent-constant.ll (added)
>> +++ llvm/trunk/test/Transforms/IndVarSimplify/2014-06-21-congruent-constant.ll Sat Jun 21 06:47:18 2014
>> @@ -0,0 +1,51 @@
>> +; RUN: opt -S -loop-unswitch -instcombine -indvars < %s | FileCheck %s
> 
> Hi Benjamnin,
> 
> any reason you are running -loop-unswitch and -instcombine as well, instead of using their output as test case?

I wasn't able to reproduce the crash when doing that. The PHI node seen here is essentially a nop and everyone will try to fold it away quickly so it's hard to observe in IndVarSimplify. It makes the test case less valuable because it now depends on the behavior of the other passes too :(

- Ben