[PATCH] D19549: Fold compares irrespective of whether allocation can be elided

[Anna Thomas via llvm-commits]

anna added inline comments.

================
Comment at: lib/Analysis/InstructionSimplify.cpp:2100
@@ +2099,3 @@
+  // compare dominates the pointer escape
+  if (MI && !PointerMayBeCaptured(MI, false, false))
+    return ConstantInt::get(GetCompareTy(LHS), !CmpInst::isTrueWhenEqual(Pred));
----------------
anna wrote:
> sanjoy wrote:
> > I think you need to pass `true` for both `ReturnCaptures` and `StoreCaptures`.  Can you also add a test case that would've caught this?
> I had `ReturnCaptures` as `false` because of this case:
> compare dominates the `ret` statement which returns the pointer. We can fold the compare since pointer has not escaped at the time of compare. 
> 
> In case compare does not dominate the `ret` statement, if we fold the `cmp` and some optimizations later on reordere the statements such that the `ret` no longer returns the right value, we have incorrect functionality in the IR. 
> However, the test case should be such that the ret value is different, but the only statement causing the pointer to be captured should be the `ret`. If there were other statements causing pointer to be captured, we would have returned `false` for `PointerMayBeCaptured()`
> 
> TC I wrote for the case where `cmp` for `%m` does not dominate the `ret` 
> 
> ```
> define i8* @compare_ret_escape(i8* %c) {
> 
>   %m = call i8* @malloc(i64 4)
>   %n = call i8* @malloc(i64 4)
>   %cmp = icmp eq i8* %n, %c
>   br i1 %cmp, label %retst, label %chk
> 
> retst:
>   ret i8* %m
> 
> chk:
>   %bc = bitcast i8* %m to i32*
>   %lgp = load i32*, i32** @gp, align 8, !nonnull !0
>   %cmp2 = icmp eq i32* %bc, %lgp
>   br i1 %cmp2, label %retst,  label %chk2
> 
> chk2:
>   ret i8* %n
> }
> ```
> Right now the `%cmp2` folds to `false`, and it looks right.  Is there a case where we really need `ReturnCaptures` to be `true`?
> 
> 
Also, just to add to this, the third case is `cmp` post dominates the `ret`. In all cases, the important point is that `ret` is a terminator instruction that exits the function, so there is no way to go from the `ret` to `cmp`. 


Repository:
  rL LLVM

http://reviews.llvm.org/D19549