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

[Sanjay Patel via Phabricator via llvm-commits]

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."));
   }
 }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D29404.86667.patch
Type: text/x-patch
Size: 2631 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170201/c3578c69/attachment.bin>