<div dir="ltr">You may want to emit the same warning on test cases like this:<br><div> ; CHECK-LABEL: define i32 @_Z1ik(i32 %p)</div><div> define i32 @_Z1ik(i32 %p) {</div><div> entry:</div><div> %cmp = icmp eq i32 %p, 42</div><div> call void @llvm.assume(i1 %cmp)</div><div><br></div><div> ; CHECK: br i1 true, label %bb2, label %bb3</div><div> br i1 %cmp, label %bb2, label %bb3</div><div> bb2:</div><div> ; CHECK-NOT: %cmp3 =</div><div> %cmp3 = icmp eq i32 %p, 43</div><div> ; CHECK: store i8 undef, i8* null</div><div> call void @llvm.assume(i1 %cmp3)</div><div> ret i32 15</div><div> bb3:</div><div> ret i32 17</div><div> }</div><div><br></div><div><br></div><div>(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)</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Feb 1, 2017 at 10:21 AM, Sanjay Patel via Phabricator via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">spatel created this revision.<br>
Herald added a subscriber: mcrosier.<br>
<br>
This is a follow-up to <a href="https://reviews.llvm.org/D29395" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D29395</a> where we try to be good citizens and let the user know that we've gone off the rails.<br>
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.<br>
<br>
This should allow us to resolve:<br>
<a href="https://llvm.org/bugs/show_bug.cgi?id=31809" rel="noreferrer" target="_blank">https://llvm.org/bugs/show_<wbr>bug.cgi?id=31809</a><br>
<br>
<br>
<a href="https://reviews.llvm.org/D29404" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D29404</a><br>
<br>
Files:<br>
lib/Analysis/ValueTracking.cpp<br>
test/Transforms/InstSimplify/<wbr>assume.ll<br>
<br>
<br>
Index: test/Transforms/InstSimplify/<wbr>assume.ll<br>
==============================<wbr>==============================<wbr>=======<br>
--- test/Transforms/InstSimplify/<wbr>assume.ll<br>
+++ test/Transforms/InstSimplify/<wbr>assume.ll<br>
@@ -1,5 +1,10 @@<br>
; NOTE: Assertions have been autogenerated by update_test_checks.py<br>
-; RUN: opt -instsimplify -S < %s | FileCheck %s<br>
+; RUN: opt -instsimplify -S < %s 2>&1 | FileCheck %s<br>
+<br>
+; Verify that warnings are emitted for the 2nd and 3rd tests.<br>
+<br>
+; CHECK: warning: <unknown>:0:0: Detected conflicting code assumptions.<br>
+; CHECK: warning: <unknown>:0:0: Detected conflicting code assumptions.<br>
<br>
define void @test1() {<br>
; CHECK-LABEL: @test1(<br>
@@ -28,8 +33,7 @@<br>
<br>
; Similar to above: there's no way to know which assumption is truthful,<br>
; so just don't crash. The second icmp+assume gets processed later, so that<br>
-; determines the return value. This can be improved by permanently invalidating<br>
-; the cached assumptions for this function.<br>
+; determines the return value.<br>
<br>
define i8 @conflicting_assumptions(i8 %x) {<br>
; CHECK-LABEL: @conflicting_assumptions(<br>
Index: lib/Analysis/ValueTracking.cpp<br>
==============================<wbr>==============================<wbr>=======<br>
--- lib/Analysis/ValueTracking.cpp<br>
+++ lib/Analysis/ValueTracking.cpp<br>
@@ -25,6 +25,7 @@<br>
#include "llvm/IR/ConstantRange.h"<br>
#include "llvm/IR/Constants.h"<br>
#include "llvm/IR/DataLayout.h"<br>
+#include "llvm/IR/DiagnosticInfo.h"<br>
#include "llvm/IR/Dominators.h"<br>
#include "llvm/IR/<wbr>GetElementPtrTypeIterator.h"<br>
#include "llvm/IR/GlobalAlias.h"<br>
@@ -791,19 +792,16 @@<br>
<br>
// If assumptions conflict with each other or previous known bits, then we<br>
// have a logical fallacy. This should only happen when a program has<br>
- // undefined behavior. We can't assert/crash, so clear out the known bits and<br>
- // hope for the best.<br>
-<br>
- // FIXME: Publish a warning/remark that we have encountered UB or the compiler<br>
- // is broken.<br>
-<br>
- // FIXME: Implement a stronger version of "I give up" by invalidating/clearing<br>
- // the assumption cache. This should indicate that the cache is corrupted so<br>
- // future callers will not waste time repopulating it with faulty assumptions.<br>
-<br>
+ // undefined behavior. We can't assert/crash, so clear out the known bits,<br>
+ // warn the user, and hope for the best.<br>
if ((KnownZero & KnownOne) != 0) {<br>
KnownZero.clearAllBits();<br>
KnownOne.clearAllBits();<br>
+<br>
+ Q.CxtI->getContext().diagnose(<wbr>DiagnosticInfoOptimizationFail<wbr>ure(<br>
+ *Q.CxtI->getFunction(), Q.CxtI->getDebugLoc(),<br>
+ "Detected conflicting code assumptions. Program has undefined behavior "<br>
+ "or internal compiler error."));<br>
}<br>
}<br>
<br>
<br>
<br>
<br>______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div>