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

[spatel]

Summary

This is a follow-up to 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

[davide]

lib/Analysis/ValueTracking.cpp

800–804 ↗

(On Diff #86667)

I'm struggling here. It's true we're emitting a diagnostic related to the fact we can't optimize, but it's also true we're not optimizing because the IR to which we lowered exhibits undefined behaviour (so, there's a correctness issue).
Also, I thought we could use OptimizationRemarkEmitter for this kind of tasks? (+ Adam) @anemet

test/Transforms/InstSimplify/assume.ll

4–7 ↗

(On Diff #86667)

Can we actually get a test to make sure we emit the correct debuginfo/debugloc instead of <unknown> ?

[anemet]

lib/Analysis/ValueTracking.cpp

800–804 ↗

(On Diff #86667)

I am not sure this should be an optimization failure. I think (@hfinkel, correct me if I am wrong), we use failure for cases where the user has explicitly asked us to optimization the code in a way that we fail to do. I.e. it's a pretty strong diagnostics -- almost like a warning.

In this case I am not sure how severe this message should be. We can emit a missed optimization remark or an analysis remark, the latter being the catch-all and usually the most noisy.

In terms of the API used, this is the legacy API that we're trying to move away from. It would be great to convert instsimplify to require OptimizationRemarkEmitter analysis and then stash that somewhere (in Q perhaps?!) and then use the new ORE->emit(remark) interface.

[spatel]

Patch updated:

1. Use newer OptimizationRemarkEmitter API (old API is not marked deprecated?)
2. Use OptimizationRemarkMissed().
3. Updated tests to include debug info so it appears in the CHECK lines.
4. Removed independent changes about invalidating the cache (rL293823).

[davide]

I think this patch has still some problems (but please correct me if I'm wrong).
Here you're adding a dependency on OptimizationRemarkEmitter but, isn't that supposed to be used as an analysis?
If so, your pass should probably request it (and you should stick the ORE somewhere).

[spatel]

lib/Analysis/ValueTracking.cpp

800–804 ↗

(On Diff #86667)

I've updated as suggested, but this doesn't seem right to me (I have never touched this API before this, so it's likely I haven't used this as intended...).

When we hit this case, we're headed for disaster: either we have a bug in LLVM or the program has UB. Either way, it's unlikely that the customer is going to be happy with the output. Is "OptimizationRemarkMissed" strong enough? I thought something equivalent to a stern warning would be appreciated here.

This API requires a pass name, but this is just value tracking, so we don't know which pass is actually getting us into here?

[davide]

lib/Analysis/ValueTracking.cpp

800–804 ↗

(On Diff #86667)

I think what Adam is actually suggesting (or well, at least what I'm actually suggesting) is to make InstCombine/InstSimplify dependent on the OptimizationRemarksEmittedAnalysis (assuming we really want to go this route, it may make sense or not).

[anemet]

lib/Analysis/ValueTracking.cpp

800–804 ↗

(On Diff #86667)

When we hit this case, we're headed for disaster: either we have a bug in LLVM or the program has UB. Either way, it's unlikely that the customer is going to be happy with the output. Is "OptimizationRemarkMissed" strong enough? I thought something equivalent to a stern warning would be appreciated here.

OK, so go back to failure. Sorry, I haven't been following the discussion. Failure is I think surfaced as a backend warning. You might want to check. The vectorizer emits those for loops where vectorization was requested by a pragma but we couldn't do it.

This API requires a pass name, but this is just value tracking, so we don't know which pass is actually getting us into here?

I don't think that using value-tracking would cause any issues for now. On the other hand the second parameter is an identifier and we use camel-cased single word.

[hfinkel]

lib/Analysis/ValueTracking.cpp

800–804 ↗

(On Diff #86667)

OK, so go back to failure. Sorry, I haven't been following the discussion. Failure is I think surfaced as a backend warning. You might want to check. The vectorizer emits those for loops where vectorization was requested by a pragma but we couldn't do it.

I agree.

[anemet]

lib/Analysis/ValueTracking.cpp

800–804 ↗

(On Diff #86667)

Turns out I never ported the vectorizer to use OptimizationFailure via ORE. rL293866 fixes this omission.

[spatel]

Patch updated:
Use DiagnosticInfoOptimizationFailure again because we do want a warning here.

But this isn't behaving the way I expected: shouldn't this show up as a "warning" rather than a "remark" and without needing to specify "-pass-remarks-missed=.*"? Ie, DS_Warning was specified in the creation of the underlying DiagnosticInfoIROptimization().

Sorry if I'm still mis-using the APIs...

[anemet]

In D29404#664884, @spatel wrote:

But this isn't behaving the way I expected: shouldn't this show up as a "warning" rather than a "remark" and without needing to specify "-pass-remarks-missed=.*"? Ie, DS_Warning was specified in the creation of the underlying DiagnosticInfoIROptimization().

Yes, it should:

$ ./bin/opt -loop-vectorize ../test/Transforms/LoopVectorize/X86/vectorization-remarks-missed.ll -o /dev/null
remark: source.cpp:19:5: loop not vectorized: cannot identify array bounds
warning: source.cpp:19:5: loop not vectorized: failed explicitly specified loop vectorization

The remark part is special-cased in LV to always appear but I think the warning is supposed to show up regardless.

Also once you resolve this part, you still need to change the patch to use ORE as an analysis pass via InstSimplify.

[efriedma]

When we hit this case, we're headed for disaster: either we have a bug in LLVM or the program has UB.

"the program has UB" is a bit ambiguous. It's possible the "llvm.assume" call is dynamically unreachable, and therefore the undefined behavior doesn't matter. The dynamically unreachable codepath might not even exist in the original source code; for example, loop unswitching could duplicate a loop, and then one of "llvm.assume" calls becomes unreachable due to a correlated condition. This is one of the reasons LLVM doesn't emit warnings when the optimizer discovers code with undefined behavior.

[spatel]

In D29404#664955, @efriedma wrote:

When we hit this case, we're headed for disaster: either we have a bug in LLVM or the program has UB.

"the program has UB" is a bit ambiguous. It's possible the "llvm.assume" call is dynamically unreachable, and therefore the undefined behavior doesn't matter. The dynamically unreachable codepath might not even exist in the original source code; for example, loop unswitching could duplicate a loop, and then one of "llvm.assume" calls becomes unreachable due to a correlated condition. This is one of the reasons LLVM doesn't emit warnings when the optimizer discovers code with undefined behavior.

Ah, I didn't think of that possibility. In that case, abandon patch?

[spatel]

Patch updated:

1. Given Eli's observation about the potential to hit this case validly, soften this back to a OptimizationRemarkAnalysis() and use wishy-washy words in the remark.
2. Add SimplifyInstructions dependency on OptimizationRemarkEmitter and stash the ORE pointer in the Query struct as suggested. I've tried to minimize the diffs by making the ORE an optional param, but this still seems to require a lot of code. If I understand correctly, we would have to do this for all passes that use value tracking and update all calls into value tracking to guarantee that a remark is printed.

[anemet]

This LGTM now both in terms of the usage of ORE and the nature of the diagnostic. It would be good to have someone else also sign off on the choice of the diagnostics though.

lib/Analysis/ValueTracking.cpp

80 ↗

(On Diff #86974)

Say that this could really be null (unlike other analyses) because not all clients provide it currently.

[efriedma]

It seems fine to emit an optimization remark here. I don't recall the details about the different levels of optimization remarks off the top of my head, though.

llvm/trunk/include/llvm/Analysis/InstructionSimplify.h

 namespace llvm {
   template<typename T>
   class ArrayRef;
   class AssumptionCache;
   class DominatorTree;
   class Instruction;
   class DataLayout;
   class FastMathFlags;
+  class OptimizationRemarkEmitter;
   class TargetLibraryInfo;
   class Type;
   class Value;
 
   /// Given operands for an Add, fold the result or return null.
   Value *SimplifyAddInst(Value *LHS, Value *RHS, bool isNSW, bool isNUW,
                          const DataLayout &DL,
                          const TargetLibraryInfo *TLI = nullptr,
Value *SimplifyCall(Value *V, ArrayRef<Value *> Args, const DataLayout &DL,
                       AssumptionCache *AC = nullptr,
                       const Instruction *CxtI = nullptr);
 
   /// See if we can compute a simplified version of this instruction. If not,
   /// return null.
   Value *SimplifyInstruction(Instruction *I, const DataLayout &DL,
                              const TargetLibraryInfo *TLI = nullptr,
                              const DominatorTree *DT = nullptr,
-                             AssumptionCache *AC = nullptr);
+                             AssumptionCache *AC = nullptr,
+                             OptimizationRemarkEmitter *ORE = nullptr);
 
   /// Replace all uses of 'I' with 'SimpleV' and simplify the uses recursively.
   ///
   /// This first performs a normal RAUW of I with SimpleV. It then recursively
   /// attempts to simplify those users updated by the operation. The 'I'
   /// instruction must not be equal to the simplified value 'SimpleV'.
   ///
   /// The function returns true if any simplifications were performed.

llvm/trunk/include/llvm/Analysis/ValueTracking.h

template <typename T> class ArrayRef;
   class AddOperator;
   class AssumptionCache;
   class DataLayout;
   class DominatorTree;
   class GEPOperator;
   class Instruction;
   class Loop;
   class LoopInfo;
+  class OptimizationRemarkEmitter;
   class MDNode;
   class StringRef;
   class TargetLibraryInfo;
   class Value;
 
   namespace Intrinsic {
   enum ID : unsigned;
   }
 
   /// Determine which bits of V are known to be either zero or one and return
   /// them in the KnownZero/KnownOne bit sets.
   ///
   /// This function is defined on values with integer type, values with pointer
   /// type, and vectors of integers.  In the case
   /// where V is a vector, the known zero and known one values are the
   /// same width as the vector element, and the bit is set only if it is true
   /// for all of the elements in the vector.
   void computeKnownBits(const Value *V, APInt &KnownZero, APInt &KnownOne,
                         const DataLayout &DL, unsigned Depth = 0,
                         AssumptionCache *AC = nullptr,
                         const Instruction *CxtI = nullptr,
-                        const DominatorTree *DT = nullptr);
+                        const DominatorTree *DT = nullptr,
+                        OptimizationRemarkEmitter *ORE = nullptr);
   /// Compute known bits from the range metadata.
   /// \p KnownZero the set of bits that are known to be zero
   /// \p KnownOne the set of bits that are known to be one
   void computeKnownBitsFromRangeMetadata(const MDNode &Ranges,
                                          APInt &KnownZero, APInt &KnownOne);
   /// Return true if LHS and RHS have no common bits set.
   bool haveNoCommonBitsSet(const Value *LHS, const Value *RHS,
                            const DataLayout &DL,

llvm/trunk/lib/Analysis/InstructionSimplify.cpp

 
 #include "llvm/Analysis/InstructionSimplify.h"
 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/AliasAnalysis.h"
 #include "llvm/Analysis/CaptureTracking.h"
 #include "llvm/Analysis/ConstantFolding.h"
 #include "llvm/Analysis/MemoryBuiltins.h"
+#include "llvm/Analysis/OptimizationDiagnosticInfo.h"
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/Analysis/VectorUtils.h"
 #include "llvm/IR/ConstantRange.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/GetElementPtrTypeIterator.h"
 #include "llvm/IR/GlobalAlias.h"
 #include "llvm/IR/Operator.h"
Value *llvm::SimplifyCall(Value *V, ArrayRef<Value *> Args,
   return ::SimplifyCall(V, Args.begin(), Args.end(),
                         Query(DL, TLI, DT, AC, CxtI), RecursionLimit);
 }
 
 /// See if we can compute a simplified version of this instruction.
 /// If not, this returns null.
 Value *llvm::SimplifyInstruction(Instruction *I, const DataLayout &DL,
                                  const TargetLibraryInfo *TLI,
-                                 const DominatorTree *DT, AssumptionCache *AC) {
+                                 const DominatorTree *DT, AssumptionCache *AC,
+                                 OptimizationRemarkEmitter *ORE) {
   Value *Result;
 
   switch (I->getOpcode()) {
   default:
     Result = ConstantFoldInstruction(I, DL, TLI);
     break;
   case Instruction::FAdd:
     Result = SimplifyFAddInst(I->getOperand(0), I->getOperand(1),
#undef HANDLE_CAST_INST
   }
 
   // In general, it is possible for computeKnownBits to determine all bits in a
   // value even when the operands are not all constants.
   if (!Result && I->getType()->isIntOrIntVectorTy()) {
     unsigned BitWidth = I->getType()->getScalarSizeInBits();
     APInt KnownZero(BitWidth, 0);
     APInt KnownOne(BitWidth, 0);
-    computeKnownBits(I, KnownZero, KnownOne, DL, /*Depth*/0, AC, I, DT);
+    computeKnownBits(I, KnownZero, KnownOne, DL, /*Depth*/0, AC, I, DT, ORE);
     if ((KnownZero | KnownOne).isAllOnesValue())
       Result = ConstantInt::get(I->getType(), KnownOne);
   }
 
   /// If called on unreachable code, the above logic may report that the
   /// instruction simplified to itself.  Make life easier for users by
   /// detecting that case here, returning a safe value instead.
   return Result == I ? UndefValue::get(I->getType()) : Result;

llvm/trunk/lib/Analysis/ValueTracking.cpp

 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/Analysis/AssumptionCache.h"
 #include "llvm/Analysis/InstructionSimplify.h"
 #include "llvm/Analysis/MemoryBuiltins.h"
 #include "llvm/Analysis/Loads.h"
 #include "llvm/Analysis/LoopInfo.h"
+#include "llvm/Analysis/OptimizationDiagnosticInfo.h"
 #include "llvm/Analysis/VectorUtils.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/ConstantRange.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/GetElementPtrTypeIterator.h"
 #include "llvm/IR/GlobalAlias.h"

 // context (the context instruction provides that context). If an assume and
 // the context instruction are not in the same block then the DT helps in
 // figuring out if we can use it.
 struct Query {
   const DataLayout &DL;
   AssumptionCache *AC;
   const Instruction *CxtI;
   const DominatorTree *DT;
+  // Unlike the other analyses, this may be a nullptr because not all clients
+  // provide it currently.
+  OptimizationRemarkEmitter *ORE;
 
   /// Set of assumptions that should be excluded from further queries.
   /// This is because of the potential for mutual recursion to cause
   /// computeKnownBits to repeatedly visit the same assume intrinsic. The
   /// classic case of this is assume(x = y), which will attempt to determine
   /// bits in x from bits in y, which will attempt to determine bits in y from
   /// bits in x, etc. Regarding the mutual recursion, computeKnownBits can call
   /// isKnownNonZero, which calls computeKnownBits and ComputeSignBit and
   /// isKnownToBeAPowerOfTwo (all of which can call computeKnownBits), and so
   /// on.
   std::array<const Value *, MaxDepth> Excluded;
   unsigned NumExcluded;
 
   Query(const DataLayout &DL, AssumptionCache *AC, const Instruction *CxtI,
-        const DominatorTree *DT)
-      : DL(DL), AC(AC), CxtI(CxtI), DT(DT), NumExcluded(0) {}
+        const DominatorTree *DT, OptimizationRemarkEmitter *ORE = nullptr)
+      : DL(DL), AC(AC), CxtI(CxtI), DT(DT), ORE(ORE), NumExcluded(0) {}
 
   Query(const Query &Q, const Value *NewExcl)
-      : DL(Q.DL), AC(Q.AC), CxtI(Q.CxtI), DT(Q.DT), NumExcluded(Q.NumExcluded) {
+      : DL(Q.DL), AC(Q.AC), CxtI(Q.CxtI), DT(Q.DT), ORE(Q.ORE),
+        NumExcluded(Q.NumExcluded) {
     Excluded = Q.Excluded;
     Excluded[NumExcluded++] = NewExcl;
     assert(NumExcluded <= Excluded.size());
   }
 
   bool isExcluded(const Value *Value) const {
     if (NumExcluded == 0)
       return false;

 }
 
 static void computeKnownBits(const Value *V, APInt &KnownZero, APInt &KnownOne,
                              unsigned Depth, const Query &Q);
 
 void llvm::computeKnownBits(const Value *V, APInt &KnownZero, APInt &KnownOne,
                             const DataLayout &DL, unsigned Depth,
                             AssumptionCache *AC, const Instruction *CxtI,
-                            const DominatorTree *DT) {
+                            const DominatorTree *DT,
+                            OptimizationRemarkEmitter *ORE) {
   ::computeKnownBits(V, KnownZero, KnownOne, Depth,
-                     Query(DL, AC, safeCxtI(V, CxtI), DT));
+                     Query(DL, AC, safeCxtI(V, CxtI), DT, ORE));
 }
 
 bool llvm::haveNoCommonBitsSet(const Value *LHS, const Value *RHS,
                                const DataLayout &DL,
                                AssumptionCache *AC, const Instruction *CxtI,
                                const DominatorTree *DT) {
   assert(LHS->getType() == RHS->getType() &&
          "LHS and RHS should have the same type");
// assume(v <_u c)
           APInt::getHighBitsSet(BitWidth, RHSKnownZero.countLeadingOnes()+1);
       else
         KnownZero |=
           APInt::getHighBitsSet(BitWidth, RHSKnownZero.countLeadingOnes());
     }
   }
 
   // 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.
-
+  // have a logical fallacy. It's possible that the assumption is not reachable,
+  // so this isn't a real bug. On the other hand, the program may have undefined
+  // behavior, or we might have a bug in the compiler. We can't assert/crash, so
+  // clear out the known bits, try to warn the user, and hope for the best.
   if ((KnownZero & KnownOne) != 0) {
     KnownZero.clearAllBits();
     KnownOne.clearAllBits();
+
+    if (Q.ORE) {
+      auto *CxtI = const_cast<Instruction *>(Q.CxtI);
+      OptimizationRemarkAnalysis ORA("value-tracking", "BadAssumption", CxtI);
+      Q.ORE->emit(ORA << "Detected conflicting code assumptions. Program may "
+                         "have undefined behavior, or compiler may have "
+                         "internal error.");
+    }
   }
 }
 
 // Compute known bits from a shift operator, including those with a
 // non-constant shift amount. KnownZero and KnownOne are the outputs of this
 // function. KnownZero2 and KnownOne2 are pre-allocated temporaries with the
 // same bit width as KnownZero and KnownOne. KZF and KOF are operator-specific
 // functors that, given the known-zero or known-one bits respectively, and a

llvm/trunk/lib/Transforms/Utils/SimplifyInstructions.cpp

 //===----------------------------------------------------------------------===//
 
 #include "llvm/Transforms/Utils/SimplifyInstructions.h"
 #include "llvm/ADT/DepthFirstIterator.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/AssumptionCache.h"
 #include "llvm/Analysis/InstructionSimplify.h"
+#include "llvm/Analysis/OptimizationDiagnosticInfo.h"
 #include "llvm/Analysis/TargetLibraryInfo.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/Type.h"
 #include "llvm/Pass.h"
 #include "llvm/Transforms/Utils/Local.h"
 #include "llvm/Transforms/Scalar.h"
 using namespace llvm;
 
 #define DEBUG_TYPE "instsimplify"
 
 STATISTIC(NumSimplified, "Number of redundant instructions removed");
 
 static bool runImpl(Function &F, const DominatorTree *DT,
-                    const TargetLibraryInfo *TLI, AssumptionCache *AC) {
+                    const TargetLibraryInfo *TLI, AssumptionCache *AC,
+                    OptimizationRemarkEmitter *ORE) {
   const DataLayout &DL = F.getParent()->getDataLayout();
   SmallPtrSet<const Instruction *, 8> S1, S2, *ToSimplify = &S1, *Next = &S2;
   bool Changed = false;
 
   do {
     for (BasicBlock *BB : depth_first(&F.getEntryBlock())) {
       // Here be subtlety: the iterator must be incremented before the loop
       // body (not sure why), so a range-for loop won't work here.
       for (BasicBlock::iterator BI = BB->begin(), BE = BB->end(); BI != BE;) {
         Instruction *I = &*BI++;
         // The first time through the loop ToSimplify is empty and we try to
         // simplify all instructions.  On later iterations ToSimplify is not
         // empty and we only bother simplifying instructions that are in it.
         if (!ToSimplify->empty() && !ToSimplify->count(I))
           continue;
 
         // Don't waste time simplifying unused instructions.
         if (!I->use_empty()) {
-          if (Value *V = SimplifyInstruction(I, DL, TLI, DT, AC)) {
+          if (Value *V = SimplifyInstruction(I, DL, TLI, DT, AC, ORE)) {
             // Mark all uses for resimplification next time round the loop.
             for (User *U : I->users())
               Next->insert(cast<Instruction>(U));
             I->replaceAllUsesWith(V);
             ++NumSimplified;
             Changed = true;
           }
         }
InstSimplifier() : FunctionPass(ID) {
       initializeInstSimplifierPass(*PassRegistry::getPassRegistry());
     }
 
     void getAnalysisUsage(AnalysisUsage &AU) const override {
       AU.setPreservesCFG();
       AU.addRequired<DominatorTreeWrapperPass>();
       AU.addRequired<AssumptionCacheTracker>();
       AU.addRequired<TargetLibraryInfoWrapperPass>();
+      AU.addRequired<OptimizationRemarkEmitterWrapperPass>();
     }
 
     /// runOnFunction - Remove instructions that simplify.
     bool runOnFunction(Function &F) override {
       if (skipFunction(F))
         return false;
 
       const DominatorTree *DT =
           &getAnalysis<DominatorTreeWrapperPass>().getDomTree();
       const TargetLibraryInfo *TLI =
           &getAnalysis<TargetLibraryInfoWrapperPass>().getTLI();
       AssumptionCache *AC =
           &getAnalysis<AssumptionCacheTracker>().getAssumptionCache(F);
-      return runImpl(F, DT, TLI, AC);
+      OptimizationRemarkEmitter *ORE =
+          &getAnalysis<OptimizationRemarkEmitterWrapperPass>().getORE();
+
+      return runImpl(F, DT, TLI, AC, ORE);
     }
   };
 }
 
 char InstSimplifier::ID = 0;
 INITIALIZE_PASS_BEGIN(InstSimplifier, "instsimplify",
                       "Remove redundant instructions", false, false)
 INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)
 INITIALIZE_PASS_DEPENDENCY(DominatorTreeWrapperPass)
 INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(OptimizationRemarkEmitterWrapperPass)
 INITIALIZE_PASS_END(InstSimplifier, "instsimplify",
                     "Remove redundant instructions", false, false)
 char &llvm::InstructionSimplifierID = InstSimplifier::ID;
 
 // Public interface to the simplify instructions pass.
 FunctionPass *llvm::createInstructionSimplifierPass() {
   return new InstSimplifier();
 }
 
 PreservedAnalyses InstSimplifierPass::run(Function &F,
                                       FunctionAnalysisManager &AM) {
   auto &DT = AM.getResult<DominatorTreeAnalysis>(F);
   auto &TLI = AM.getResult<TargetLibraryAnalysis>(F);
   auto &AC = AM.getResult<AssumptionAnalysis>(F);
-  bool Changed = runImpl(F, &DT, &TLI, &AC);
+  auto &ORE = AM.getResult<OptimizationRemarkEmitterAnalysis>(F);
+  bool Changed = runImpl(F, &DT, &TLI, &AC, &ORE);
   if (!Changed)
     return PreservedAnalyses::all();
 
   PreservedAnalyses PA;
   PA.preserveSet<CFGAnalyses>();
   return PA;
 }

llvm/trunk/test/Transforms/InstSimplify/assume.ll

 ; NOTE: Assertions have been autogenerated by update_test_checks.py
-; RUN: opt -instsimplify -S < %s | FileCheck %s
+; RUN: opt -instsimplify -S < %s 2>&1 -pass-remarks-analysis=.* | FileCheck %s
+
+; Verify that warnings are emitted for the 2nd and 3rd tests.
+
+; CHECK: remark: /tmp/s.c:1:13: Detected conflicting code assumptions.
+; CHECK: remark: /tmp/s.c:4:10: Detected conflicting code assumptions.
 
 define void @test1() {
 ; CHECK-LABEL: @test1(
 ; CHECK:         ret void
 ;
   call void @llvm.assume(i1 1)
   ret void
 
 }
 
 ; The alloca guarantees that the low bits of %a are zero because of alignment.
 ; The assume says the opposite. The assume is processed last, so that's the 
 ; return value. There's no way to win (we can't undo transforms that happened
 ; based on half-truths), so just don't crash.
 
-define i64 @PR31809() {
+define i64 @PR31809() !dbg !7 {
 ; CHECK-LABEL: @PR31809(
 ; CHECK-NEXT:    ret i64 3
 ;
   %a = alloca i32
-  %t1 = ptrtoint i32* %a to i64
+  %t1 = ptrtoint i32* %a to i64, !dbg !9
   %cond = icmp eq i64 %t1, 3
   call void @llvm.assume(i1 %cond)
   ret i64 %t1
 }
 
 ; 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.
 
-define i8 @conflicting_assumptions(i8 %x) {
+define i8 @conflicting_assumptions(i8 %x) !dbg !10 {
 ; CHECK-LABEL: @conflicting_assumptions(
 ; CHECK-NEXT:    call void @llvm.assume(i1 false)
 ; CHECK-NEXT:    [[COND2:%.*]] = icmp eq i8 %x, 4
 ; CHECK-NEXT:    call void @llvm.assume(i1 [[COND2]])
 ; CHECK-NEXT:    ret i8 5
 ;
-  %add = add i8 %x, 1
+  %add = add i8 %x, 1, !dbg !11
   %cond1 = icmp eq i8 %x, 3
   call void @llvm.assume(i1 %cond1)
   %cond2 = icmp eq i8 %x, 4
   call void @llvm.assume(i1 %cond2)
   ret i8 %add
 }
 
 declare void @llvm.assume(i1) nounwind
 
+!llvm.dbg.cu = !{!0}
+!llvm.module.flags = !{!3, !4, !5}
+!llvm.ident = !{!6}
+
+!0 = distinct !DICompileUnit(language: DW_LANG_C99, file: !1, producer: "clang version 4.0.0 (trunk 282540) (llvm/trunk 282542)", isOptimized: true, runtimeVersion: 0, emissionKind: LineTablesOnly, enums: !2)
+!1 = !DIFile(filename: "/tmp/s.c", directory: "/tmp")
+!2 = !{}
+!3 = !{i32 2, !"Dwarf Version", i32 4}
+!4 = !{i32 2, !"Debug Info Version", i32 3}
+!5 = !{i32 1, !"PIC Level", i32 2}
+!6 = !{!"clang version 4.0.0 (trunk 282540) (llvm/trunk 282542)"}
+!7 = distinct !DISubprogram(name: "foo", scope: !1, file: !1, line: 1, type: !8, isLocal: false, isDefinition: true, scopeLine: 1, isOptimized: true, unit: !0, variables: !2)
+!8 = !DISubroutineType(types: !2)
+!9 = !DILocation(line: 1, column: 13, scope: !7)
+!10 = distinct !DISubprogram(name: "bar", scope: !1, file: !1, line: 3, type: !8, isLocal: false, isDefinition: true, scopeLine: 3, isOptimized: true, unit: !0, variables: !2)
+!11 = !DILocation(line: 4, column: 10, scope: !10)
+!12 = !DILocation(line: 4, column: 3, scope: !10)
+