[PATCH] D30446: [IndVars] Do not branch on poison

Sanjoy Das via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 16 10:05:50 PDT 2017


sanjoy added a comment.

In https://reviews.llvm.org/D30446#702790, @atrick wrote:

> When I talk about creating or removing an IV, I'm talking about the strongly connected phi cycle. I'm not referring to any "derived IV" stemming from values that depend on the IV.
>
> I found the dependent patches and roughly figured out what's hapenning here. It seems like we could leave the original IV's `nw` flags in place and simply introduce an `add` without the `nuw` flags for the branch's post-increment. Maybe that's what Eli is suggesting? I think that would result in a nicely canonical form, but it may require adjusting downstream passes. I think the risk of interfering with downstream passes could be reduced by moving all of LFTR into LSR.


It isn't just that, but also cases like this:

  target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
  target triple = "x86_64-unknown-linux-gnu"
  
  ;; Slightly edited version of the @f_1 function in the test case, but as I've noted
  ;; the test case in the patch is broken and needs to be fixed to be the same as this.
  define i32 @f_1(i32* %ptr, i1 %always_false) {
  entry:
    br label %for.cond
  
  for.cond:
    %iv = phi i32 [ -2147483648, %entry ], [ %iv.inc, %always_taken ]
    %iv2 = phi i32 [ 0, %entry ], [ %iv2.inc, %always_taken ]
    store i32 %iv, i32* %ptr
    br i1 %always_false, label %never_taken, label %always_taken
  
  never_taken:
    store volatile i32 %iv2, i32* %ptr
    br label %always_taken
  
  always_taken:
    %iv.inc = add nsw i32 %iv, 1
    %iv2.inc = add nsw i32 %iv2, 1
    %cmp = icmp slt i32 %iv, 20
    br i1 %cmp, label %for.cond, label %for.end
  
  for.end:
    ret i32 0
  }

where LFTR rewrites the exit condition to use `%iv2.inc` (a "different" induction variable) which may be poison (and is poison in the last two iterations in this case).

Hanging the comparison off of a non-nsw/nuw increment of `%iv2` won't help since by the last iteration `%iv2` is poison, so a non-nsw/nuw addition will propagate that poison.



================
Comment at: test/Transforms/IndVarSimplify/PR31181.ll:50
+always_taken:
+  %iv.inc = add nuw nsw i32 %iv, 1
+  %iv2.inc = add nuw nsw i32 %iv2, 1
----------------
This test case is wrong -- it the increment on `%iv.inc` should just be `nsw` and not `nuw`.


https://reviews.llvm.org/D30446





More information about the llvm-commits mailing list