[PATCH] D51582: [IndVars] Turn isValidRewrite into an assertion

[Andrew Trick via Phabricator via llvm-commits]

atrick added a comment.

The bitcast bailout (implemented in 2009) (https://github.com/llvm-mirror/llvm/blob/master/lib/Analysis/ScalarEvolution.cpp#L6451)

wasn't catching this case (fixed in 2011). The problem had to do with LCSSA preventing normal SCEV simplification...

Before:

  define i8* @stdimpl_itoa(i32 %v, i8* %string, i32 %r) nounwind {
  entry:
    %buf.i = alloca [34 x i8], align 4
  ...
  bb4.i12.preheader:                                ; preds = %bb.i4                                                                                                                                      
    %.lcssa20 = phi i8* [ %21, %bb.i4 ]
    br label %bb4.i12
  
  bb4.i12:                                          ; preds = %bb4.i12, %bb4.i12.preheader                                                                                                                
    %start.0.i10 = phi i8* [ %26, %bb4.i12 ], [ %string, %bb4.i12.preheader ]
    %p.1.i11 = phi i8* [ %24, %bb4.i12 ], [ %.lcssa20, %bb4.i12.preheader ]
  ...
    store i8 %25, i8* %start.0.i10, align 1
    %26 = getelementptr inbounds i8* %start.0.i10, i32 1
    %27 = icmp eq i8* %16, %24
    br i1 %27, label %_ZL5_xtoamPcii.exit14, label %bb4.i12
  
  _ZL5_xtoamPcii.exit14:                            ; preds = %bb4.i12                                                                                                                                    
    %.lcssa19 = phi i8* [ %26, %bb4.i12 ]
    store i8 0, i8* %.lcssa19, align 1
    ret i8* %string

Notice the definition of %26, the phi in the exit block and the store address.

After:

  bb4.i12.preheader:                                ; preds = %bb.i4                                                                                                                                      
    %.lcssa20 = phi i8* [ %scevgep31, %bb.i4 ]
  ...
  _ZL5_xtoamPcii.exit14:                            ; preds = %bb4.i12                                                                                                                                    
    %scevgep18 = getelementptr i8* %.lcssa20, i32 %scevgep1617
    store i8 0, i8* %scevgep18, align 1
    ret i8* %string

The address is effectively %scevgep31 which is defined in bb.i4:
%scevgep31 = getelementptr [34 x i8]* %buf.i, i32 0, i32 %tmp30

%buf.i is an alloca so DSE will end up deleting the store.

---

So, maybe it's best to leave the assertion.


https://reviews.llvm.org/D51582