[PATCH] D63928: [ARM][SCEV][LSR] Prevent using undefined value in binops

[Eugene Leviant via Phabricator via llvm-commits]

evgeny777 added a comment.

Thanks for looking at it @samparker. I'll try to reduce the test case, but it's pretty hard to do this, because of lots of heuristics in LSR.
Even when I use different target (say i386) problem stops reproducing.

I'll try to explain a bit further. The problem arises when SCEV expander tries to evaluate `((-1 * %7) + %9)` (see lsr-undef-in-binop.ll) which should result to zero, but
becomes undefined because while expanding `(-1 * %7)` value `%9` is converted to `%10 = ptrtoint i8* undef to i32`. The two values `%8 = ptrtoint i8* %7 to i32` which
is evaluated `%7` and `%10 = ptrtoint i8* undef to i32` (evaluated `%9`) are passed to `InsertBinop(Instruction::Sub, ...)` (see visitAddExpr).

This results in broken IR after running opt -S -loop-reduce:

  %8 = ptrtoint i8* %7 to i32
  ; ...
  %10 = ptrtoint i8* undef to i32  ; %10 is undefined
  ; ...
  
  .preheader:
  %120 = sub i32 %10, %8  ; the result of this binary operation is undefined
  %121 = sub i32 %120, %5  ; the result is undefined as well
  %scevgep8 = getelementptr i8, i8* %118, i32 %121  ; value of scevgep8 is undefined
  ; ...
  
  ; the loop below corresponds to construct_backwards called from std::vector and inlined
  124:                                              ; preds = %.preheader, %124
    %lsr.iv = phi i32 [ 0, %.preheader ], [ %lsr.iv.next, %124 ]
  ; ..... loop body (skipped)
    %lsr.iv.next = add i32 %lsr.iv, -1
    %tmp = inttoptr i32 %lsr.iv.next to i8*
    %126 = icmp eq i8* %scevgep8, %tmp  ; The result of this comparison is undefined. On my device it runs until overwrites random memory location and program crashes
    br i1 %126, label %127, label %124

My patch attempts to fix the issue by not making cast instruction undefined. If you apply the patch you'll get:

  %8 = ptrtoint i8* %7 to i32
  ; ...
  %10 = ptrtoint i8* undef to i32  ; %10 is undefined

All values depending on %10 become defined.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63928/new/

https://reviews.llvm.org/D63928