[PATCH] D29404: [ValueTracking] emit a warning when we detect a conflicting assumption (PR31809)

[Daniel Berlin via llvm-commits]

You may want to emit the same warning on test cases like this:
 ; CHECK-LABEL: define i32 @_Z1ik(i32 %p)
 define i32 @_Z1ik(i32 %p) {
 entry:
   %cmp = icmp eq i32 %p, 42
   call void @llvm.assume(i1 %cmp)

   ; CHECK: br i1 true, label %bb2, label %bb3
   br i1 %cmp, label %bb2, label %bb3
 bb2:
   ; CHECK-NOT: %cmp3 =
   %cmp3 = icmp eq i32 %p, 43
   ; CHECK: store i8 undef, i8* null
   call void @llvm.assume(i1 %cmp3)
   ret i32 15
 bb3:
   ret i32 17
 }


(note that if you propagate the result of cmp, you think the second assume
is false. I'd say anywhere we prove an assume to be both false, and true,
you should emit this warning :P)

On Wed, Feb 1, 2017 at 10:21 AM, Sanjay Patel via Phabricator via
llvm-commits <llvm-commits at lists.llvm.org> wrote:

> spatel created this revision.
> Herald added a subscriber: mcrosier.
>
> This is a follow-up to https://reviews.llvm.org/D29395 where we try to be
> good citizens and let the user know that we've gone off the rails.
> As discussed previously, I'm also removing the comment about invalidating
> the assumption cache because that's unlikely to help and just adds more
> code where we don't want it.
>
> This should allow us to resolve:
> https://llvm.org/bugs/show_bug.cgi?id=31809
>
>
> https://reviews.llvm.org/D29404
>
> Files:
>   lib/Analysis/ValueTracking.cpp
>   test/Transforms/InstSimplify/assume.ll
>
>
> Index: test/Transforms/InstSimplify/assume.ll
> ===================================================================
> --- test/Transforms/InstSimplify/assume.ll
> +++ test/Transforms/InstSimplify/assume.ll
> @@ -1,5 +1,10 @@
>  ; NOTE: Assertions have been autogenerated by update_test_checks.py
> -; RUN: opt -instsimplify -S < %s | FileCheck %s
> +; RUN: opt -instsimplify -S < %s 2>&1 | FileCheck %s
> +
> +; Verify that warnings are emitted for the 2nd and 3rd tests.
> +
> +; CHECK: warning: <unknown>:0:0: Detected conflicting code assumptions.
> +; CHECK: warning: <unknown>:0:0: Detected conflicting code assumptions.
>
>  define void @test1() {
>  ; CHECK-LABEL: @test1(
> @@ -28,8 +33,7 @@
>
>  ; Similar to above: there's no way to know which assumption is truthful,
>  ; so just don't crash. The second icmp+assume gets processed later, so
> that
> -; determines the return value. This can be improved by permanently
> invalidating
> -; the cached assumptions for this function.
> +; determines the return value.
>
>  define i8 @conflicting_assumptions(i8 %x) {
>  ; CHECK-LABEL: @conflicting_assumptions(
> Index: lib/Analysis/ValueTracking.cpp
> ===================================================================
> --- lib/Analysis/ValueTracking.cpp
> +++ lib/Analysis/ValueTracking.cpp
> @@ -25,6 +25,7 @@
>  #include "llvm/IR/ConstantRange.h"
>  #include "llvm/IR/Constants.h"
>  #include "llvm/IR/DataLayout.h"
> +#include "llvm/IR/DiagnosticInfo.h"
>  #include "llvm/IR/Dominators.h"
>  #include "llvm/IR/GetElementPtrTypeIterator.h"
>  #include "llvm/IR/GlobalAlias.h"
> @@ -791,19 +792,16 @@
>
>    // If assumptions conflict with each other or previous known bits, then
> we
>    // have a logical fallacy. This should only happen when a program has
> -  // undefined behavior. We can't assert/crash, so clear out the known
> bits and
> -  // hope for the best.
> -
> -  // FIXME: Publish a warning/remark that we have encountered UB or the
> compiler
> -  // is broken.
> -
> -  // FIXME: Implement a stronger version of "I give up" by
> invalidating/clearing
> -  // the assumption cache. This should indicate that the cache is
> corrupted so
> -  // future callers will not waste time repopulating it with faulty
> assumptions.
> -
> +  // undefined behavior. We can't assert/crash, so clear out the known
> bits,
> +  // warn the user, and hope for the best.
>    if ((KnownZero & KnownOne) != 0) {
>      KnownZero.clearAllBits();
>      KnownOne.clearAllBits();
> +
> +    Q.CxtI->getContext().diagnose(DiagnosticInfoOptimizationFailure(
> +        *Q.CxtI->getFunction(), Q.CxtI->getDebugLoc(),
> +        "Detected conflicting code assumptions. Program has undefined
> behavior "
> +        "or internal compiler error."));
>    }
>  }
>
>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170201/806b3bf3/attachment.html>