From: Sanjay Patel Date: Mon, 6 Feb 2017 18:26:06 +0000 (+0000) Subject: [ValueTracking] emit a remark when we detect a conflicting assumption (PR31809) X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=f6e615cc841a5fc63c1895d7eff9f581fc8ad059;p=llvm [ValueTracking] emit a remark when we detect a conflicting assumption (PR31809) This is a follow-up to D29395 where we try to be good citizens and let the user know that we've probably gone off the rails. This should allow us to resolve: https://llvm.org/bugs/show_bug.cgi?id=31809 Differential Revision: https://reviews.llvm.org/D29404 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@294208 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/llvm/Analysis/InstructionSimplify.h b/include/llvm/Analysis/InstructionSimplify.h index 47d6118313c..e4ffa1c0362 100644 --- a/include/llvm/Analysis/InstructionSimplify.h +++ b/include/llvm/Analysis/InstructionSimplify.h @@ -42,6 +42,7 @@ namespace llvm { class Instruction; class DataLayout; class FastMathFlags; + class OptimizationRemarkEmitter; class TargetLibraryInfo; class Type; class Value; @@ -296,7 +297,8 @@ namespace llvm { 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. /// diff --git a/include/llvm/Analysis/ValueTracking.h b/include/llvm/Analysis/ValueTracking.h index 5121c8b0c17..9b055250d0c 100644 --- a/include/llvm/Analysis/ValueTracking.h +++ b/include/llvm/Analysis/ValueTracking.h @@ -31,6 +31,7 @@ template class ArrayRef; class Instruction; class Loop; class LoopInfo; + class OptimizationRemarkEmitter; class MDNode; class StringRef; class TargetLibraryInfo; @@ -52,7 +53,8 @@ template class ArrayRef; 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 diff --git a/lib/Analysis/InstructionSimplify.cpp b/lib/Analysis/InstructionSimplify.cpp index fc0b86aaa39..2c7ca6210ad 100644 --- a/lib/Analysis/InstructionSimplify.cpp +++ b/lib/Analysis/InstructionSimplify.cpp @@ -24,6 +24,7 @@ #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" @@ -4452,7 +4453,8 @@ Value *llvm::SimplifyCall(Value *V, ArrayRef Args, /// 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()) { @@ -4601,7 +4603,7 @@ Value *llvm::SimplifyInstruction(Instruction *I, const DataLayout &DL, 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); } diff --git a/lib/Analysis/ValueTracking.cpp b/lib/Analysis/ValueTracking.cpp index 26032e70320..d8e56768da5 100644 --- a/lib/Analysis/ValueTracking.cpp +++ b/lib/Analysis/ValueTracking.cpp @@ -20,6 +20,7 @@ #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" @@ -76,6 +77,9 @@ struct Query { 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 @@ -90,11 +94,12 @@ struct Query { 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()); @@ -131,9 +136,10 @@ static void computeKnownBits(const Value *V, APInt &KnownZero, APInt &KnownOne, 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, @@ -790,16 +796,21 @@ static void computeKnownBitsFromAssume(const Value *V, APInt &KnownZero, } // 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(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."); + } } } diff --git a/lib/Transforms/Utils/SimplifyInstructions.cpp b/lib/Transforms/Utils/SimplifyInstructions.cpp index 432a2c5479b..f6070868de4 100644 --- a/lib/Transforms/Utils/SimplifyInstructions.cpp +++ b/lib/Transforms/Utils/SimplifyInstructions.cpp @@ -20,6 +20,7 @@ #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" @@ -35,7 +36,8 @@ using namespace llvm; 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 S1, S2, *ToSimplify = &S1, *Next = &S2; bool Changed = false; @@ -54,7 +56,7 @@ static bool runImpl(Function &F, const DominatorTree *DT, // 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(U)); @@ -95,6 +97,7 @@ namespace { AU.addRequired(); AU.addRequired(); AU.addRequired(); + AU.addRequired(); } /// runOnFunction - Remove instructions that simplify. @@ -108,7 +111,10 @@ namespace { &getAnalysis().getTLI(); AssumptionCache *AC = &getAnalysis().getAssumptionCache(F); - return runImpl(F, DT, TLI, AC); + OptimizationRemarkEmitter *ORE = + &getAnalysis().getORE(); + + return runImpl(F, DT, TLI, AC, ORE); } }; } @@ -119,6 +125,7 @@ INITIALIZE_PASS_BEGIN(InstSimplifier, "instsimplify", 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; @@ -133,7 +140,8 @@ PreservedAnalyses InstSimplifierPass::run(Function &F, auto &DT = AM.getResult(F); auto &TLI = AM.getResult(F); auto &AC = AM.getResult(F); - bool Changed = runImpl(F, &DT, &TLI, &AC); + auto &ORE = AM.getResult(F); + bool Changed = runImpl(F, &DT, &TLI, &AC, &ORE); if (!Changed) return PreservedAnalyses::all(); diff --git a/test/Transforms/InstSimplify/assume.ll b/test/Transforms/InstSimplify/assume.ll index 4255238126b..66f2120f292 100644 --- a/test/Transforms/InstSimplify/assume.ll +++ b/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 -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( @@ -15,12 +20,12 @@ define void @test1() { ; 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 @@ -30,14 +35,14 @@ define i64 @PR31809() { ; 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 @@ -47,3 +52,21 @@ define i8 @conflicting_assumptions(i8 %x) { 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) +